[Octave-patch-tracker] [patch #9067] java sources part 1

[Ernst Reissner]

Follow-up Comment #7, patch #9067 (project octave):

Hi Mike, 
I tried not to make trailing spaces (as I am used to do). 
Some places I found and corrected. 
Now it should be correct. 

Concerning indentation, now I applied my auto-indenter... 
should be ok now. 

Break of function arguments: I also corrected. 

Now I deleted code that was commented out. 

NOPMD: I use a coding style checker detecting weak coding, 
called PMD. 
Sometimes it is better to break rules, 
but this must be documented. 
NOPMD is a marker for PMD program to suspend checking. 

In the new patch I replaced 'NOPMD' 
by 'NOPMD: rulecheck suspended'
If this is not acceptable to you, please let me know. 

The place with else if is probably e.g. in method setStaticField. 
I think code is better now because I have problems 
finding the if for the else... Hm.. 
maybe a matter of habit. 
Maybe it is better because the then branch consists of nothing but the return.

I must insist, that the last if i deleted is an improvement. 
;-)

What do you mean by 'please fold those changes into and refresh the existing
patch'? 

Shall I provide a patch with all changes? 
You do not want a follow up patch with only the corrections to the path,
right? 

Concerning your last point: 
I tried not to change functionality at all. 
Just simplified and documented. 

An exception is method addClassPath, 
which is now based on OctClassLoader.addUrl, 
no longer on OctClassLoader.addClassPath. 
Thus I could remove this method, which is no longer required. 

What concerns exceptions or more general Throwables, 
I did two things: 
I declared more precisely what kind of Exceptions could be thrown. 
Throwable is the base class, subclasses are Exception and Error. 
It is bad style just to declare Exception, 
because this can be everything. 
So I specified the subclass tht can be thrown as precisely as possible. 
Also I documented the cases in which that kind of exception can be thrown. 

An example is removeClassPath: 
No longer Exception is thrown, but MalformedURLException. 
Also it is declared that this cannot occur, 
if the parameter path is really a path. 
Also it is documented in the body of the method, 
where exactly the exception could be thrown. 

The second thing I did, 
is that I generally eliminated empty catches, 
because they are considered bad style hiding misbehavior. 
This is the case for getClassPath(). 
Now the exception is not silently dropped, 
but thrown. 
In this case, I gave an explanation, 
why this exception cannnot occur, 
although declared. 

I think, I can explain in all places, 
that i did not change functionality. 

If you have questions, please ask. 

I have the impression, you do not feel so very comfortable 
with my changes, 
maybe because you do not have any tests at hand. 
I could provide, but only for internal use, 
an arithmetics defined in java using the interface. 

Are you interested in that? 


Answering your other post: 
I fully understand your point, that precedence is: 

reformat/redesign < documentation < feature < bugfix, 

but sometimes 
reformat/redesign and documentation prior to bugfix 
eases review of bugfix considerably. 

I hope this is the case here. 






    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?9067>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/