[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15283629#comment-15283629 ] Gilles commented on MATH-758: - Can we resolve this issue? It is relatively old. Some concrete cases reported have been handled. Others have disappeared with their deprecated classes. In some cases, the problem runs deeper (cf. MATH-1228). The general principle is always valid, and should be integrated in the design unless there is reason (to be documented) not to. It would be clearer to list each remaining issue of this type in its own JIRA report. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Improvement >Reporter: Sebb > Fix For: 4.0 > > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[
https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13222661#comment-13222661
]
Leandro Ariel Pezzente commented on MATH-758:
-
{quote} I must point again that those issues stem from not having a consistent
coding policy, e.g. a rule such as "It's never necessary to expose mutable data
fields directly". This rule should be integrated when thinking about the
implementation, not as an after-thought.{quote}
Would it be feasible to integrate a "Design Docs" section into Apache Commons
Math Wiki Site
> Fields which could be private and/or final
> --
>
> Key: MATH-758
> URL: https://issues.apache.org/jira/browse/MATH-758
> Project: Commons Math
> Issue Type: Bug
>Reporter: Sebb
>
> BaseAbstractUnivariateIntegrator has several fields that are not currently
> changed after construction and could be final:
> protected double absoluteAccuracy;
> protected double relativeAccuracy;
> protected int minimalIterationCount;
> protected Incrementor iterations;
> protected Incrementor evaluations;
> These all have getters as well, so could also be made private.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221579#comment-13221579 ] Luc Maisonobe commented on MATH-758: I confirm the use of protected variables in ODE is a design choice. It was driven by performance considerations and the need to change data in place at several levels and avoid copying as much as possible. This is also the reason why tha API provides array as placeholders that must be filled in by the equation implementation. If the API were redesigned now, with modern JVM that handle this far better than a few years ago, of course the design decisions would not be the same. However, this API is heavily used and would require changes in all user code, so I would not do it a few days before a release. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221511#comment-13221511 ] Gilles commented on MATH-758: - I think that most of the easy stuff has been done. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221490#comment-13221490 ] Gilles commented on MATH-758: - I totally agree on the data encapsulation issue. The problem is the timing: It is not the appropriate time to make large modifications to the code because almost 2 years of work must released. Most of these issues are going to stay at the level of "potential problems" (the field are not intended to be directly accessed by users) as they have been until now. I must point again that those issues stem from not having a consistent coding policy, e.g. a rule such as "It's never necessary to expose mutable data fields directly". This rule should be integrated when thinking about the implementation, not as an after-thought. Redesigning some of the affected implementations will need more time than I have now. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221480#comment-13221480 ] Sebb commented on MATH-758: --- bq. Anyways, my impression is that it cannot be changed without a lot of work -> no change at this point. If it's not changed now, then even more work will likely be needed if it is done later. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221479#comment-13221479 ] Sebb commented on MATH-758: --- Note that getters can return the array without copying (though returning the array itself is best avoided if indexed getters can be used instead). Using a getter instead of direct access means that it would be possible at a later date to change the code to clone the array; this is not possible when direct access is allowed. For sub-classes that access array elements, then a getter(int idx) that gets the element would be preferable, as it makes the element read-only. This should be applicable in the AbstractWell case. It's never *necessary* to expose mutable data fields directly, so the default should be for all such fields to be private, with access via getters (and setters if necessary). > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221477#comment-13221477 ] Gilles commented on MATH-758: - There is a heavy use of protected variables in the classes of the "ode" package. Maybe that Luc can give his opinion as to whether it has to be so. Anyways, my impression is that it cannot be changed without a lot of work -> no change at this point. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221471#comment-13221471 ] Gilles commented on MATH-758: - The logic of "AbstractWell" (package "random") relies on the variables being "protected" -> no change at this point. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221470#comment-13221470 ] Gilles commented on MATH-758: - bq. LaguerreSolver.complexSolver - could this be private and final? Done. > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221466#comment-13221466 ] Gilles commented on MATH-758: - Class "AbstractLeastSquaresOptimizer" in package "optimization.general" uses many protected instance variables which are modified in subclasses. This cannot be changed at this point (too many things to touch and a quite likely performance loss due to array copying). > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (MATH-758) Fields which could be private and/or final
[ https://issues.apache.org/jira/browse/MATH-758?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221445#comment-13221445 ] Sebb commented on MATH-758: --- LaguerreSolver.complexSolver - could this be private and final? Dfp fields exp/mant/nans/sign - looks like these could be private with getters, apart from nans which is written by subclasses. AbstractFormat.setDenominatorFormat() AbstractFormat.setNumeratorFormat() - are these needed (only used in test code)? If not, the underlying fields could be made final NordsieckStepInterpolator.stateVariation is only used internally AFAICT, so could be private FirstMoment.n has a getter, and is not updated externally, so could be private FourthMoment.m4 is only read externally by Kurtosis, so could be private (there is a getter) The fields m1, m2, m3 are only ever written by the xxxMoment classes, which set them to zero. Each class could have a protected zero() method, which could be chained in the same way as the clear() method. Other classes could then use the getters and m1-m4 could be private. [To be continued] > Fields which could be private and/or final > -- > > Key: MATH-758 > URL: https://issues.apache.org/jira/browse/MATH-758 > Project: Commons Math > Issue Type: Bug >Reporter: Sebb > > BaseAbstractUnivariateIntegrator has several fields that are not currently > changed after construction and could be final: > protected double absoluteAccuracy; > protected double relativeAccuracy; > protected int minimalIterationCount; > protected Incrementor iterations; > protected Incrementor evaluations; > These all have getters as well, so could also be made private. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
