You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gilles Sadowski <gi...@harfang.homelinux.org> on 2010/11/14 21:57:34 UTC

Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Hi Luc.

> Author: luc
> Date: Sun Nov 14 15:06:11 2010
> New Revision: 1035003
> 
> URL: http://svn.apache.org/viewvc?rev=1035003&view=rev
> Log:
> fixed message building so the behavior matches the existing javadoc:
> general arguments are the ones remaining after the specific ones have been handled

I'm afraid that it was intended the other way around! A message log must
come out as:

<problem_general_description>: <specific_context_description>

Although I did try doing something along your changes, I did not pursue it
because it would mess the existing enums: it makes it impossible to use the
same parameters for both the general description and the specific ones!

For example a general problem is "DimensionMismatchException"; and it is
clear how to format a "general" message that describes this problem and for
this, you use up two parameters which are
 1. the wrong dimension
 2. the expected dimension
If a more "specific" message is provided, it would still need these two
parameters, but they are gone now!

Doing it the other way around, as I think was the purpose of your change
(and of your question about "MathUserException") still has the same
shortcoming: "specific" uses up the parameters and they are not available
anymore for "general".
I think that a more flexible framework would be necessary to accomodate
that functionality (which I didn't want to suggest since a large overhaul
would again ensue).

The implementation in fact allowed redundant message, where "redundant"
refers to having the parameters appearing more than once.
With your change, the redundancy will appear at the point where the
exception is instantiated. E.g. if SPECIFIC_PATTERN would use two
parameters, one would need
  new DimensionMismatchException(SPECIFIC_PATTERN, 1, 2, 1, 2);
while with the other constructor, it must be:
  new DimensionMismatchException(1, 2);

I think that this is highly undesirable.


Sorry for not having written a clearer class documentation that would have
mentions this caveat.

Best,
Gilles

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


Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.

> [...]
> 
> So what I propose the following set of constructors :
>  - no argument
>      -> this would lead to the default message USER_EXCEPTION
>  - a throwable
>      -> this would lead to the default message USER_EXCEPTION
>  - a throwable and one pattern
>      -> this would be used to specify the generic part
>  - a throwable and two patterns
>      -> this would be used to specify both the generic
>          and the specific part

As I said, this is similar to the code of "MathIllegalArgumentException".
That's fine.

> In fact, all these constructors end up calling the last one, by adding
> some null arguments. All arguments can be null.

No, IMHO, the "general" parameter should not be null.
If a user defines a class that inherit from "MathUserException", the meaning
will exist and should be reflected in the "general" pattern.
[Allowing null is suggesting to use this "tool" in a wrong way.]

However, this is really nitpicking; the result will just be useless empty
error message, and that will be the user's fault. So, allow "null" if you
like ;-)

> [...]


Best regards,
Gilles

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


Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 14/11/2010 23:22, Gilles Sadowski a écrit :
> Hi.

Hi,


> 
>>> [...]
>>>
>>> I'll revert the change and make the javadoc match the earlier code.
>>
>> It should be fixed now, in r1035072 and r1035073.
> 
> Thank you.
> 
>> I left the other change (allowing either specific or general to be null)
>> as I still consider its worth providing both ways.
> 
> I don't understand. IMHO, it doesn't make sense to have no general
> description: if an exception exists, it has a meaning; it is that meaning
> that must be conveyed in the "general" description (in effect it is
> redundant with the exception's type; but that was your requirement: a
> localized message that some applications could print without referring to
> the stack trace.

You are right, but for a user exception, it's not commons-math that
knows the meaning, but the user. So the user should have access to the
general specifier. For now, we hide this access and provide him only the
specific part.

> 
> Moreover, by default the error message will be as follows:
> 
> <exception type>: <problem_general_description>[: <specific_context_description>]
> 
> where only the last part is optional (it will not be there if there is no
> "specific" pattern.
> 
> In the case of "MathUserException" (as in most other cases), the "specific"
> part is provided at the instantiation of the exception object, while the
> "general" part is tied to the class definition (cf. above). Only the base
> classes of the exception provide constructors with 2 Localizable arguments,
> and they should rarely, if ever, be instantiated directly.

We are in the very rare case where the type of the exception
(MathUserException) does *not* provide information. All information (and
it is very important for the user) is either in a wrapped exception
(IMHO it is the best use), or in the message. I just want to let the
user provide what he wants when he build his instance, not bothering at
commons-math level about it (i.e. not adding extra parts without his
consent) and letting him recover his instance untouched at top level.

> 
>> So my question on
>> MATH-440 is still valid and I would like to remove the general stuff
>> from user message, to make sure we don't mess with what the user want.
> 
> I answered there, but obviously I'm still missing something (either what the
> problem is, or how to explain to you that there is no problem).
> 
> So, to summarize, I think that the "general" pattern certainly cannot be
> null.
> However, if there is a case that users would want to base a hierarchy of
> exceptions on "MathUserException", then it should provide an additional
> constructor with 2 "Localizable".

Yes. My last comment in MATH-44 is wrong by the way, so I continue here
as the list is more suited for discussion. It IS possible to have a
non-ambiguous set of constructors despite the variable arguments. In
fact, it is already what we have in MessageFactory with the two static
methods buildMessage.

So what I propose the following set of constructors :
 - no argument
     -> this would lead to the default message USER_EXCEPTION
 - a throwable
     -> this would lead to the default message USER_EXCEPTION
 - a throwable and one pattern
     -> this would be used to specify the generic part
 - a throwable and two patterns
     -> this would be used to specify both the generic
         and the specific part

In fact, all these constructors end up calling the last one, by adding
some null arguments. All arguments can be null.


> I think that the rest of the code should be left as it was.

Yes, I only add constructors to a class which provides some boilerplate
for users and which commons-math does not use by itself. So this affects
strictly nothing in commons-math, it only improves user control.

best regards,
Luc

> 
> 
> 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


Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.

> > [...]
> > 
> > I'll revert the change and make the javadoc match the earlier code.
> 
> It should be fixed now, in r1035072 and r1035073.

Thank you.

> I left the other change (allowing either specific or general to be null)
> as I still consider its worth providing both ways.

I don't understand. IMHO, it doesn't make sense to have no general
description: if an exception exists, it has a meaning; it is that meaning
that must be conveyed in the "general" description (in effect it is
redundant with the exception's type; but that was your requirement: a
localized message that some applications could print without referring to
the stack trace.

Moreover, by default the error message will be as follows:

<exception type>: <problem_general_description>[: <specific_context_description>]

where only the last part is optional (it will not be there if there is no
"specific" pattern.

In the case of "MathUserException" (as in most other cases), the "specific"
part is provided at the instantiation of the exception object, while the
"general" part is tied to the class definition (cf. above). Only the base
classes of the exception provide constructors with 2 Localizable arguments,
and they should rarely, if ever, be instantiated directly.

> So my question on
> MATH-440 is still valid and I would like to remove the general stuff
> from user message, to make sure we don't mess with what the user want.

I answered there, but obviously I'm still missing something (either what the
problem is, or how to explain to you that there is no problem).

So, to summarize, I think that the "general" pattern certainly cannot be
null.
However, if there is a case that users would want to base a hierarchy of
exceptions on "MathUserException", then it should provide an additional
constructor with 2 "Localizable".
I think that the rest of the code should be left as it was.


Regards,
Gilles

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


Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 14/11/2010 22:19, Luc Maisonobe a écrit :
> Le 14/11/2010 21:57, Gilles Sadowski a écrit :
>> Hi Luc.
> 
> Hi Gilles,
> 
>>
>>> Author: luc
>>> Date: Sun Nov 14 15:06:11 2010
>>> New Revision: 1035003
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1035003&view=rev
>>> Log:
>>> fixed message building so the behavior matches the existing javadoc:
>>> general arguments are the ones remaining after the specific ones have been handled
>>
>> I'm afraid that it was intended the other way around! A message log must
>> come out as:
>>
>> <problem_general_description>: <specific_context_description>
>>
>> Although I did try doing something along your changes, I did not pursue it
>> because it would mess the existing enums: it makes it impossible to use the
>> same parameters for both the general description and the specific ones!
> 
> Then the javadoc was wrong. I tried to get the code consistent with it
> by changing the code. We can get consistency by changing the javadoc ...
> 
>>
>> For example a general problem is "DimensionMismatchException"; and it is
>> clear how to format a "general" message that describes this problem and for
>> this, you use up two parameters which are
>>  1. the wrong dimension
>>  2. the expected dimension
>> If a more "specific" message is provided, it would still need these two
>> parameters, but they are gone now!
> 
> Yes.
> 
>>
>> Doing it the other way around, as I think was the purpose of your change
>> (and of your question about "MathUserException") still has the same
>> shortcoming: "specific" uses up the parameters and they are not available
>> anymore for "general".
>> I think that a more flexible framework would be necessary to accomodate
>> that functionality (which I didn't want to suggest since a large overhaul
>> would again ensue).
> 
> The framework is already complex enough, I would not like to have it
> more complex.
> 
>>
>> The implementation in fact allowed redundant message, where "redundant"
>> refers to having the parameters appearing more than once.
>> With your change, the redundancy will appear at the point where the
>> exception is instantiated. E.g. if SPECIFIC_PATTERN would use two
>> parameters, one would need
>>   new DimensionMismatchException(SPECIFIC_PATTERN, 1, 2, 1, 2);
>> while with the other constructor, it must be:
>>   new DimensionMismatchException(1, 2);
>>
>> I think that this is highly undesirable.
>>
>>
>> Sorry for not having written a clearer class documentation that would have
>> mentions this caveat.
> 
> I'll revert the change and make the javadoc match the earlier code.

It should be fixed now, in r1035072 and r1035073.

I left the other change (allowing either specific or general to be null)
as I still consider its worth providing both ways. So my question on
MATH-440 is still valid and I would like to remove the general stuff
from user message, to make sure we don't mess with what the user want.

Luc

> 
> Sorry for having followed the wrong track.
> 
> Luc
> 
>>
>> Best,
>> 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
> 


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


Re: svn commit: r1035003 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math/exception/util/MessageFactory.java test/java/org/apache/commons/math/exception/util/MessageFactoryTest.java

Posted by Luc Maisonobe <Lu...@free.fr>.
Le 14/11/2010 21:57, Gilles Sadowski a écrit :
> Hi Luc.

Hi Gilles,

> 
>> Author: luc
>> Date: Sun Nov 14 15:06:11 2010
>> New Revision: 1035003
>>
>> URL: http://svn.apache.org/viewvc?rev=1035003&view=rev
>> Log:
>> fixed message building so the behavior matches the existing javadoc:
>> general arguments are the ones remaining after the specific ones have been handled
> 
> I'm afraid that it was intended the other way around! A message log must
> come out as:
> 
> <problem_general_description>: <specific_context_description>
> 
> Although I did try doing something along your changes, I did not pursue it
> because it would mess the existing enums: it makes it impossible to use the
> same parameters for both the general description and the specific ones!

Then the javadoc was wrong. I tried to get the code consistent with it
by changing the code. We can get consistency by changing the javadoc ...

> 
> For example a general problem is "DimensionMismatchException"; and it is
> clear how to format a "general" message that describes this problem and for
> this, you use up two parameters which are
>  1. the wrong dimension
>  2. the expected dimension
> If a more "specific" message is provided, it would still need these two
> parameters, but they are gone now!

Yes.

> 
> Doing it the other way around, as I think was the purpose of your change
> (and of your question about "MathUserException") still has the same
> shortcoming: "specific" uses up the parameters and they are not available
> anymore for "general".
> I think that a more flexible framework would be necessary to accomodate
> that functionality (which I didn't want to suggest since a large overhaul
> would again ensue).

The framework is already complex enough, I would not like to have it
more complex.

> 
> The implementation in fact allowed redundant message, where "redundant"
> refers to having the parameters appearing more than once.
> With your change, the redundancy will appear at the point where the
> exception is instantiated. E.g. if SPECIFIC_PATTERN would use two
> parameters, one would need
>   new DimensionMismatchException(SPECIFIC_PATTERN, 1, 2, 1, 2);
> while with the other constructor, it must be:
>   new DimensionMismatchException(1, 2);
> 
> I think that this is highly undesirable.
> 
> 
> Sorry for not having written a clearer class documentation that would have
> mentions this caveat.

I'll revert the change and make the javadoc match the earlier code.

Sorry for having followed the wrong track.

Luc

> 
> Best,
> 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