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 2012/03/01 15:16:11 UTC
[Math] New warnings from "FindBugs"
Hi.
The version upgrade of the FindBugs plugin led to new discoveries some of
which are potentially serious bugs:
* org.apache.commons.math3.ode.nonstiff.GraggBulirschStoerIntegrator: Was
method "setStepsizeControl" (note the spelling) intended to override
"setStepSizeControl" defined in the parent class?
* org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
probably unnecessary "Serializable"...
* org.apache.commons.math3.genetics.Chromosome: At line 31 (and 43, where
FindBugs warns about strict floating point comparison), is the value
intended to be the most negative one, instead of "Double.MIN_VALUE" (which
is positive)?
* org.apache.commons.math3.random.EmpiricalDistribution (lines 213, 221,
242, 246) and org.apache.commons.math3.random.ValueServer (line 270):
From "FindBugs":
Dm: Reliance on default encoding (DM_DEFAULT_ENCODING)
Found a call to a method which will perform a byte to String (or String to
byte) conversion, and will assume that the default platform encoding is
suitable. This will cause the application behaviour to vary between
platforms. Use an alternative API and specify a charset name or Charset
object explicitly.
I think that the last one could be fixed later (unless someone knows how to
solve it). But for the others, please have look ASAP.
Thanks,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
> >> >
> >> > >
> >> > > * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> >> > > probably unnecessary "Serializable"...
> >> > >
> >> > In fact, it comes from a nested class which extends EventObject, so it
> >> > must be (unfortunately) serializable. I'll look into it.
> >>
> >> Then, it seems that you can define it as "static", and it will make FindBugs
> >> happy, I think. [The problem is that it is a "plain" inner class, in a
> >> non-serializable class.]
> >>
> >
> > I've performed this change and also had to make the "state" field
> > "transient" because "State" is not "Serializable" and it cannot be
> > since "RealLinearOperator" is not "Serializable" and cannot be (IIUC).[1]
> > [At first sight, all this confirms that we should stay away from
> > "Serializable". :-}]
> >
>
> I do agree with you on this, although my point of view is far less
> enlightened than yours. I know "serializable" is difficult,
> problematic and error-prone, so I just keep away from it as a general
> rule. I should therefore apologize for having carelessly implemented
> serializable in this particular instance.
I don't know whether I am enlightened :-). Maybe just lazy, as in not
wanting to put more burden than necessary on a too small development team...
> > [...]
> >
>
> Actually, I *do* want to handle it differently (I've even set a TODO
> tag in the source). In short, I've used references a lot in order to
> avoid creating new objects, but this makes event handling a bit
> intricate, and I want to refactor that part (keeping the public API
> unchanged).
Yes, that does not look too clean. ;-)
Best,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Sébastien Brisard <se...@m4x.org>.
>
> Hello Sébastien.
>
Hi Gilles,
>> >
>> > >
>> > > * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
>> > > probably unnecessary "Serializable"...
>> > >
>> > In fact, it comes from a nested class which extends EventObject, so it
>> > must be (unfortunately) serializable. I'll look into it.
>>
>> Then, it seems that you can define it as "static", and it will make FindBugs
>> happy, I think. [The problem is that it is a "plain" inner class, in a
>> non-serializable class.]
>>
>
> I've performed this change and also had to make the "state" field
> "transient" because "State" is not "Serializable" and it cannot be
> since "RealLinearOperator" is not "Serializable" and cannot be (IIUC).[1]
> [At first sight, all this confirms that we should stay away from
> "Serializable". :-}]
>
I do agree with you on this, although my point of view is far less
enlightened than yours. I know "serializable" is difficult,
problematic and error-prone, so I just keep away from it as a general
rule. I should therefore apologize for having carelessly implemented
serializable in this particular instance.
>
> Best,
> Gilles
>
> [1] This quiets "FindBugs", and since that class is a private inner class,
> you'll have the possiblity to handle this in another way for 3.1.
>
Actually, I *do* want to handle it differently (I've even set a TODO
tag in the source). In short, I've used references a lot in order to
avoid creating new objects, but this makes event handling a bit
intricate, and I want to refactor that part (keeping the public API
unchanged).
Thanks for your help on this,
Sébastien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello Sébastien.
> >
> > >
> > > * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> > > probably unnecessary "Serializable"...
> > >
> > In fact, it comes from a nested class which extends EventObject, so it
> > must be (unfortunately) serializable. I'll look into it.
>
> Then, it seems that you can define it as "static", and it will make FindBugs
> happy, I think. [The problem is that it is a "plain" inner class, in a
> non-serializable class.]
>
I've performed this change and also had to make the "state" field
"transient" because "State" is not "Serializable" and it cannot be
since "RealLinearOperator" is not "Serializable" and cannot be (IIUC).[1]
[At first sight, all this confirms that we should stay away from
"Serializable". :-}]
Best,
Gilles
[1] This quiets "FindBugs", and since that class is a private inner class,
you'll have the possiblity to handle this in another way for 3.1.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
>
> >
> > * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> > probably unnecessary "Serializable"...
> >
> In fact, it comes from a nested class which extends EventObject, so it
> must be (unfortunately) serializable. I'll look into it.
Then, it seems that you can define it as "static", and it will make FindBugs
happy, I think. [The problem is that it is a "plain" inner class, in a
non-serializable class.]
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Sébastien Brisard <se...@m4x.org>.
Hi Gilles,
>
> * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> probably unnecessary "Serializable"...
>
In fact, it comes from a nested class which extends EventObject, so it
must be (unfortunately) serializable. I'll look into it.
Sébastien
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello.
>
> The version upgrade of the FindBugs plugin led to new discoveries some of
> which are potentially serious bugs:
>
> * org.apache.commons.math3.ode.nonstiff.GraggBulirschStoerIntegrator: Was
> method "setStepsizeControl" (note the spelling) intended to override
> "setStepSizeControl" defined in the parent class?
>
> * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> probably unnecessary "Serializable"...
>
> * org.apache.commons.math3.genetics.Chromosome: At line 31 (and 43, where
> FindBugs warns about strict floating point comparison), is the value
> intended to be the most negative one, instead of "Double.MIN_VALUE" (which
> is positive)?
I took care of that one.
>
> * org.apache.commons.math3.random.EmpiricalDistribution (lines 213, 221,
> 242, 246) and org.apache.commons.math3.random.ValueServer (line 270):
> From "FindBugs":
> Dm: Reliance on default encoding (DM_DEFAULT_ENCODING)
>
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
>
>
> I think that the last one could be fixed later (unless someone knows how to
> solve it). But for the others, please have look ASAP.
>
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hello Luc.
> >
> > The version upgrade of the FindBugs plugin led to new discoveries some of
> > which are potentially serious bugs:
> >
> > * org.apache.commons.math3.ode.nonstiff.GraggBulirschStoerIntegrator: Was
> > method "setStepsizeControl" (note the spelling) intended to override
> > "setStepSizeControl" defined in the parent class?
>
> No, this is a completely different method. The name and signature
> similarity was unfortunate ... I have renamed this method.
>
> Fixed in r1295772.
Thanks.
> [...]
> > * org.apache.commons.math3.random.EmpiricalDistribution (lines 213, 221,
> > 242, 246) and org.apache.commons.math3.random.ValueServer (line 270):
> > From "FindBugs":
> > Dm: Reliance on default encoding (DM_DEFAULT_ENCODING)
> >
> > Found a call to a method which will perform a byte to String (or String to
> > byte) conversion, and will assume that the default platform encoding is
> > suitable. This will cause the application behaviour to vary between
> > platforms. Use an alternative API and specify a charset name or Charset
> > object explicitly.
> >
> >
> > I think that the last one could be fixed later (unless someone knows how to
> > solve it). [...]
OK to leave those?
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [Math] New warnings from "FindBugs"
Posted by Luc Maisonobe <Lu...@free.fr>.
Le 01/03/2012 15:16, Gilles Sadowski a écrit :
> Hi.
Hi Gilles,
>
> The version upgrade of the FindBugs plugin led to new discoveries some of
> which are potentially serious bugs:
>
> * org.apache.commons.math3.ode.nonstiff.GraggBulirschStoerIntegrator: Was
> method "setStepsizeControl" (note the spelling) intended to override
> "setStepSizeControl" defined in the parent class?
No, this is a completely different method. The name and signature
similarity was unfortunate ... I have renamed this method.
Fixed in r1295772.
Sorry for the confusion.
Luc
>
> * org.apache.commons.math3.linear.SymmLQ: Yet another problem with a
> probably unnecessary "Serializable"...
>
> * org.apache.commons.math3.genetics.Chromosome: At line 31 (and 43, where
> FindBugs warns about strict floating point comparison), is the value
> intended to be the most negative one, instead of "Double.MIN_VALUE" (which
> is positive)?
>
> * org.apache.commons.math3.random.EmpiricalDistribution (lines 213, 221,
> 242, 246) and org.apache.commons.math3.random.ValueServer (line 270):
> From "FindBugs":
> Dm: Reliance on default encoding (DM_DEFAULT_ENCODING)
>
> Found a call to a method which will perform a byte to String (or String to
> byte) conversion, and will assume that the default platform encoding is
> suitable. This will cause the application behaviour to vary between
> platforms. Use an alternative API and specify a charset name or Charset
> object explicitly.
>
>
> I think that the last one could be fixed later (unless someone knows how to
> solve it). But for the others, please have look ASAP.
>
>
> Thanks,
> 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