commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: svn commit: r960602 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/analysis/interpolation/ main/java/org/apache/commons/math/exception/ main/java/org/apache/commons/math/util/ main/resources/META-INF/localization/ site/xdoc/ tes...
Date Tue, 06 Jul 2010 00:55:06 GMT
Gilles Sadowski wrote:
>>> MATH-361
>>> MATH-382
>> In the future, please separate commits if possible for separate
>> issues.  In this case, a bug was fixed and some exception
>> refactoring was also introduced.  These should be separate commits
> 
> I know, and I do it whenever possible but I found out about the bug
> (MATH-382) when I had already updated the file (MATH-361) and did not have
> time to spend rolling back the changes for something that trivial.

As I said, please separate commits in the future.  It is not that
difficult to svn revert and manage source so your local mods for
different issues can be separated.  Commit logs and diffs get
attached to issues and it makes it easier to tell what has happened
if we separate commits for different issues.

> 
>> and changes.xml should be updated with entries for each.
> 
> There is already an entry about issue 361.
> Do you really want that I duplicate that entry each time I work a little on
> this issue?

What I tend to do is create an entry in changes.xml for an issue in
progress and update it to reflect the changes associated with the
issue.  We use changes.xml as the source for the release notes, so
it should include references to all new classes, refactoring or
other significant changes to to the code.
> 
>> It would be great to discuss the exception refactoring some more.
>> The commit below gives some concrete examples.  I would like to
>> understand better why "NumberIsTooLargeException" and
>> "NumberIsTooSmallException" are better abstractions than more
>> domain-specific ones.
> 
> Of course we could have an exception package as big as the current list of
> enums:
>   NumberOfFooIsTooSmallException
>   NumberOfBarIsTooSmallException
>   etc.
> 
> However, my opinion is that an exception is supposed to tell what went
> wrong, not why. It simply cannot be a user's manual!
> 
>> Also, are we not losing information in stack
>> traces when we do:
>>
>>>          if (x.length < 2) {
>>> -            throw
>> MathRuntimeException.createIllegalArgumentException(
>>> -                  LocalizedFormats.WRONG_NUMBER_OF_POINTS, 2,
>> x.length);
>>> +            throw new NumberIsTooSmallException(x.length, 2, true);
>> Perhaps another parameter identifying the quantity that is "too
>> large" is needed?
> 
> The stack trace certainly will show where the bad thing happened.
> Nothing important is lost: What went wrong is as clearly identified by
> saying
>  "The argument must be >= 2"
> as by saying
>  "The number of points must be >= 2"
> The problem is that the caller passed a wrong argument and this is a bug and
> no amount of detailed message will be a substitute for reading the
> documentation and source code and make the correct call.

Sorry, but I disagree here.  There is definitely loss of information
in the stack trace. I would prefer to retain the detailed error
message that tells what quantity was "too small."  And I do not
really see the value in the "NumberTooSmallException" altogether,
frankly.  By substituting an exception that does not identify what
quantity was "too small" for an IllegalArgumeentException with a
message giving the full context, what exactly have we achieved?

Phil
> 
> Exceptions enhance the robustness of the library; they cannot premptively
> prevent bad usage...
> 
> 
> Best regards,
> Gilles
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message