You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2012/05/28 17:58:33 UTC
Re: svn commit: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
On 28 May 2012 16:39, <ce...@apache.org> wrote:
> Author: celestin
> Date: Mon May 28 15:39:57 2012
> New Revision: 1343293
>
> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
> Log:
> Removed explicit conversion from int to Integer.
-1; I don't think we had concensus on removing it.
>
> Modified:
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java Mon May 28 15:39:57 2012
> @@ -202,23 +202,17 @@ public abstract class RealVector {
> protected void checkIndices(final int start, final int end) {
> final int dim = getDimension();
> if ((start < 0) || (start >= dim)) {
> - throw new OutOfRangeException(LocalizedFormats.INDEX,
> - Integer.valueOf(start),
> - Integer.valueOf(0),
> - Integer.valueOf(dim - 1));
> + throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
> + dim - 1);
> }
> if ((end < 0) || (end >= dim)) {
> - throw new OutOfRangeException(LocalizedFormats.INDEX,
> - Integer.valueOf(end),
> - Integer.valueOf(0),
> - Integer.valueOf(dim - 1));
> + throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
> + dim - 1);
> }
> - if (end < start){
> + if (end < start) {
> // TODO Use more specific error message
> throw new NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
> - Integer.valueOf(end),
> - Integer.valueOf(start),
> - false);
> + end, start, false);
> }
> }
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1343293 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
On Mon, May 28, 2012 at 04:58:33PM +0100, sebb wrote:
> On 28 May 2012 16:39, <ce...@apache.org> wrote:
> > Author: celestin
> > Date: Mon May 28 15:39:57 2012
> > New Revision: 1343293
> >
> > URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
> > Log:
> > Removed explicit conversion from int to Integer.
>
> -1; I don't think we had concensus on removing it.
There was no consensus in using this notation in the first place.
There is absolutely no need for an explicit conversion in this case.
"Number" is very appropriate to store _any_ number type with the sole
expectation being that it the error message prints that number.
It's not because implicit conversion can cause problems that it always does.
And there is no need to add constructors in order to force passing
arguments of the same type: assuming that exceptions should catch wrong type
uses is like assuming that they could detect wrong variable use.
Gilles
>
> >
> > Modified:
> > commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
> >
> > Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
> > URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
> > ==============================================================================
> > --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java (original)
> > +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java Mon May 28 15:39:57 2012
> > @@ -202,23 +202,17 @@ public abstract class RealVector {
> > protected void checkIndices(final int start, final int end) {
> > final int dim = getDimension();
> > if ((start < 0) || (start >= dim)) {
> > - throw new OutOfRangeException(LocalizedFormats.INDEX,
> > - Integer.valueOf(start),
> > - Integer.valueOf(0),
> > - Integer.valueOf(dim - 1));
> > + throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
> > + dim - 1);
> > }
> > if ((end < 0) || (end >= dim)) {
> > - throw new OutOfRangeException(LocalizedFormats.INDEX,
> > - Integer.valueOf(end),
> > - Integer.valueOf(0),
> > - Integer.valueOf(dim - 1));
> > + throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
> > + dim - 1);
> > }
> > - if (end < start){
> > + if (end < start) {
> > // TODO Use more specific error message
> > throw new NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
> > - Integer.valueOf(end),
> > - Integer.valueOf(start),
> > - false);
> > + end, start, false);
> > }
> > }
> >
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Luc Maisonobe <lu...@spaceroots.org>.
+1 for implicit conversion in this case ( this is an exception message, not a result computation).
No preferences for the general case, I think it really depend on context.
Luc
--
Envoyé de mon téléphone Android avec K-9 Mail. Excusez la brièveté.
Gilles Sadowski <gi...@harfang.homelinux.org> a écrit :
> >
> That's true we didn't. However (see my last message), this is really
> of little consequence, and I didn't really want to get into a new
> "to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
> I was the one who initially defended explicit conversion as opposed to
> autoboxing, so you need not convince me. I guess most IDEs would
> signal this auto-boxing. Can we assume on this specific case that we
> are adults who know what they are doing? Again, this is just to build
> the message of an exception, so the code virtually ends here.
> I'm not familiar with -1ing a commit, I think I have to revert it
> (could you confirm on that?). I'm happy to, but please find a
> consensus with Gilles then, and I'm very happy to line up with the
> outcome of your discussions.
I don't see why there should be a consensus to remove something that was
never there in the first place. The vast majority of such exception
instantiations do not perform explicit conversion; so I'd be inclined to
think that there should be a consensus to change that.
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: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 29/05/2012 06:21, Sébastien Brisard a écrit :
> Hello,
>>
>> I don't see why there should be a consensus to remove something that was
>> never there in the first place. The vast majority of such exception
>> instantiations do not perform explicit conversion; so I'd be inclined to
>> think that there should be a consensus to change that.
>>
>>
>> Best regards,
>> Gilles
>>
> while I agree with you, I do not really know what to do: am I to
> ignore this -1 on one of my commits? This goes against the rules.
> In its latest state, the code I've committed uses auto-boxing.
+1 to revert to no explicit conversion in this specific case (i.e. it is
only an exception message, not a result computation, so it is worth
having simple code).
-1 to a general catch all rule, I guess the need for complicated code is
really context dependent.
Luc
>
> Best regards,
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Sébastien Brisard <se...@m4x.org>.
Hello,
>
> I don't see why there should be a consensus to remove something that was
> never there in the first place. The vast majority of such exception
> instantiations do not perform explicit conversion; so I'd be inclined to
> think that there should be a consensus to change that.
>
>
> Best regards,
> Gilles
>
while I agree with you, I do not really know what to do: am I to
ignore this -1 on one of my commits? This goes against the rules.
In its latest state, the code I've committed uses auto-boxing.
Best regards,
Sébastien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1343293 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
> >
> That's true we didn't. However (see my last message), this is really
> of little consequence, and I didn't really want to get into a new
> "to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
> I was the one who initially defended explicit conversion as opposed to
> autoboxing, so you need not convince me. I guess most IDEs would
> signal this auto-boxing. Can we assume on this specific case that we
> are adults who know what they are doing? Again, this is just to build
> the message of an exception, so the code virtually ends here.
> I'm not familiar with -1ing a commit, I think I have to revert it
> (could you confirm on that?). I'm happy to, but please find a
> consensus with Gilles then, and I'm very happy to line up with the
> outcome of your discussions.
I don't see why there should be a consensus to remove something that was
never there in the first place. The vast majority of such exception
instantiations do not perform explicit conversion; so I'd be inclined to
think that there should be a consensus to change that.
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: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Sébastien Brisard <se...@m4x.org>.
2012/5/28 sebb <se...@gmail.com>:
> On 28 May 2012 17:12, Sébastien Brisard <se...@m4x.org> wrote:
>> 2012/5/28 sebb <se...@gmail.com>:
>>> On 28 May 2012 16:39, <ce...@apache.org> wrote:
>>>> Author: celestin
>>>> Date: Mon May 28 15:39:57 2012
>>>> New Revision: 1343293
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
>>>> Log:
>>>> Removed explicit conversion from int to Integer.
>>>
>>> -1; I don't think we had concensus on removing it.
>>>
>>>>
>>>> Modified:
>>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>>
>>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
>>>> ==============================================================================
>>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java (original)
>>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java Mon May 28 15:39:57 2012
>>>> @@ -202,23 +202,17 @@ public abstract class RealVector {
>>>> protected void checkIndices(final int start, final int end) {
>>>> final int dim = getDimension();
>>>> if ((start < 0) || (start >= dim)) {
>>>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>>>> - Integer.valueOf(start),
>>>> - Integer.valueOf(0),
>>>> - Integer.valueOf(dim - 1));
>>>> + throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
>>>> + dim - 1);
>>>> }
>>>> if ((end < 0) || (end >= dim)) {
>>>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>>>> - Integer.valueOf(end),
>>>> - Integer.valueOf(0),
>>>> - Integer.valueOf(dim - 1));
>>>> + throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
>>>> + dim - 1);
>>>> }
>>>> - if (end < start){
>>>> + if (end < start) {
>>>> // TODO Use more specific error message
>>>> throw new NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
>>>> - Integer.valueOf(end),
>>>> - Integer.valueOf(start),
>>>> - false);
>>>> + end, start, false);
>>>> }
>>>> }
>>>>
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>> That's true we didn't. However (see my last message), this is really
>> of little consequence, and I didn't really want to get into a new
>> "to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
>> I was the one who initially defended explicit conversion as opposed to
>> autoboxing, so you need not convince me. I guess most IDEs would
>> signal this auto-boxing.
>
> Depends on the setting.
>
>> Can we assume on this specific case that we
>> are adults who know what they are doing?
>
> The way to signal that is either to use explicit boxing, or use
> @SuppressWarnings("boxing") for that method.
>
> Otherwise, at some point someone else is going to wonder whether the
> boxing is intentional or not.
>
>> Again, this is just to build
>> the message of an exception, so the code virtually ends here.
>> I'm not familiar with -1ing a commit, I think I have to revert it
>> (could you confirm on that?).
>
> The committer either has to revert the commit, or convice the person
> voting -1 to withdraw their objectioin.
>
Would adding the above anotation convince you?
>> I'm happy to, but please find a
>> consensus with Gilles then, and I'm very happy to line up with the
>> outcome of your discussions.
>
> There's another solution in this case, which is to do the boxing
> conversions in the Exceptions by providing ctors for the commonest
> Number cases (e.g. int and long). This would have the advantage that
> it would flag up mixed int and long parameters, which are probably an
> error.
>
>> Kind regards,
>> Sébastien
>>
>>
>> ---------------------------------------------------------------------
>> 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: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by sebb <se...@gmail.com>.
On 28 May 2012 17:12, Sébastien Brisard <se...@m4x.org> wrote:
> 2012/5/28 sebb <se...@gmail.com>:
>> On 28 May 2012 16:39, <ce...@apache.org> wrote:
>>> Author: celestin
>>> Date: Mon May 28 15:39:57 2012
>>> New Revision: 1343293
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
>>> Log:
>>> Removed explicit conversion from int to Integer.
>>
>> -1; I don't think we had concensus on removing it.
>>
>>>
>>> Modified:
>>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>>
>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java (original)
>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java Mon May 28 15:39:57 2012
>>> @@ -202,23 +202,17 @@ public abstract class RealVector {
>>> protected void checkIndices(final int start, final int end) {
>>> final int dim = getDimension();
>>> if ((start < 0) || (start >= dim)) {
>>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>>> - Integer.valueOf(start),
>>> - Integer.valueOf(0),
>>> - Integer.valueOf(dim - 1));
>>> + throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
>>> + dim - 1);
>>> }
>>> if ((end < 0) || (end >= dim)) {
>>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>>> - Integer.valueOf(end),
>>> - Integer.valueOf(0),
>>> - Integer.valueOf(dim - 1));
>>> + throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
>>> + dim - 1);
>>> }
>>> - if (end < start){
>>> + if (end < start) {
>>> // TODO Use more specific error message
>>> throw new NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
>>> - Integer.valueOf(end),
>>> - Integer.valueOf(start),
>>> - false);
>>> + end, start, false);
>>> }
>>> }
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
> That's true we didn't. However (see my last message), this is really
> of little consequence, and I didn't really want to get into a new
> "to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
> I was the one who initially defended explicit conversion as opposed to
> autoboxing, so you need not convince me. I guess most IDEs would
> signal this auto-boxing.
Depends on the setting.
> Can we assume on this specific case that we
> are adults who know what they are doing?
The way to signal that is either to use explicit boxing, or use
@SuppressWarnings("boxing") for that method.
Otherwise, at some point someone else is going to wonder whether the
boxing is intentional or not.
> Again, this is just to build
> the message of an exception, so the code virtually ends here.
> I'm not familiar with -1ing a commit, I think I have to revert it
> (could you confirm on that?).
The committer either has to revert the commit, or convice the person
voting -1 to withdraw their objectioin.
> I'm happy to, but please find a
> consensus with Gilles then, and I'm very happy to line up with the
> outcome of your discussions.
There's another solution in this case, which is to do the boxing
conversions in the Exceptions by providing ctors for the commonest
Number cases (e.g. int and long). This would have the advantage that
it would flag up mixed int and long parameters, which are probably an
error.
> Kind regards,
> Sébastien
>
>
> ---------------------------------------------------------------------
> 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: r1343293 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
Posted by Sébastien Brisard <se...@m4x.org>.
2012/5/28 sebb <se...@gmail.com>:
> On 28 May 2012 16:39, <ce...@apache.org> wrote:
>> Author: celestin
>> Date: Mon May 28 15:39:57 2012
>> New Revision: 1343293
>>
>> URL: http://svn.apache.org/viewvc?rev=1343293&view=rev
>> Log:
>> Removed explicit conversion from int to Integer.
>
> -1; I don't think we had concensus on removing it.
>
>>
>> Modified:
>> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java?rev=1343293&r1=1343292&r2=1343293&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/RealVector.java Mon May 28 15:39:57 2012
>> @@ -202,23 +202,17 @@ public abstract class RealVector {
>> protected void checkIndices(final int start, final int end) {
>> final int dim = getDimension();
>> if ((start < 0) || (start >= dim)) {
>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>> - Integer.valueOf(start),
>> - Integer.valueOf(0),
>> - Integer.valueOf(dim - 1));
>> + throw new OutOfRangeException(LocalizedFormats.INDEX, start, 0,
>> + dim - 1);
>> }
>> if ((end < 0) || (end >= dim)) {
>> - throw new OutOfRangeException(LocalizedFormats.INDEX,
>> - Integer.valueOf(end),
>> - Integer.valueOf(0),
>> - Integer.valueOf(dim - 1));
>> + throw new OutOfRangeException(LocalizedFormats.INDEX, end, 0,
>> + dim - 1);
>> }
>> - if (end < start){
>> + if (end < start) {
>> // TODO Use more specific error message
>> throw new NumberIsTooSmallException(LocalizedFormats.INITIAL_ROW_AFTER_FINAL_ROW,
>> - Integer.valueOf(end),
>> - Integer.valueOf(start),
>> - false);
>> + end, start, false);
>> }
>> }
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
That's true we didn't. However (see my last message), this is really
of little consequence, and I didn't really want to get into a new
"to-serialize-or-not-to-serialize-that-s-the-question" endless debate.
I was the one who initially defended explicit conversion as opposed to
autoboxing, so you need not convince me. I guess most IDEs would
signal this auto-boxing. Can we assume on this specific case that we
are adults who know what they are doing? Again, this is just to build
the message of an exception, so the code virtually ends here.
I'm not familiar with -1ing a commit, I think I have to revert it
(could you confirm on that?). I'm happy to, but please find a
consensus with Gilles then, and I'm very happy to line up with the
outcome of your discussions.
Kind regards,
Sébastien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org