Hi. > > I am a little puzzled by our use of generics in the analysis.solvers > package. > > Hoping the following ASCII art will survive mail,
It's very nice! :-) > here is a rough > overview (simplified) of what we have. > > BaseUnivariateRealSolver<FUNC> > | > +-----------------+---------------------+ > | | > | | > UnivariateRealSolver, BaseAbstractUnivariateRealSolver<FUNC> > PolynomialSolver, | > DifferentiableUnivariateRealSolver | > | | > | | > +-----------------------+---------------+ > | > | > +--------------------+-----+----------------+ > | | | > AbstractUnivariate... AbstractPolynomial... AbstractDifferentiable... > | | | > +-----+----------+ | | > | | | | > BrentSolver, BaseSecantSolver LaguerreSolver NewtonSolver > | > | > | > BaseBracketedSecantSolver > | > | > | > +--------------+---------------+ > | | | > | | | > | | | > Illinois Pegasus RegulaFalsi > > [...] > > The first point is we use "UnivariateReal" both as the name of the > topmost level type when nothing is specified (just as in the name of > the level 0 interface and level 1b abstract class), and as the name > of generic functions, in parallel with polynomial and differentiable > functions. Shouldn't we have a different name for both notions ? We > could have for example UnivariateRealFunction at top level and > GeneralRealFunction at low level. This would help separate the > meanings from level 1b and level 2. "UnivariateReal" is equivalent to your proposed "GeneralReal" "Polynomial" and "Differentiable" are sub-types. > The second point is I don't understand the purpose of interfaces > from level 1a. They are not really necessary. At first, I left them to avoid breaking compatibility. But the main usefulness is that API users can manipulate those "interface" types instead of "BaseAbstractUnivariateRealSolver<XxxFunction>" or "AbstractDifferentiableUnivariateRealSolver" which are more cumbersome to write. Also there is an asymmetry in the above two classes, due to the fact that they are not at the same level of the hierarchy. The generic base class is not supposed to be used by API users but only by CM developers (as in "AbstractDifferentiableUnivariateRealSolver"). > If on the one hand someone implements a solver by taking advantage > of the generified BaseAbstractUnivariateRealSolver we provide, these > interface merely force to add a redundant implement clause with > declarations like the ones found at level 2: > > AbstractXxxsolver extends BaseAbstractXxxSolver<XxxFunction> > implements XxxSolver > > instead of using only > > AbstractXxxsolver extends BaseAbstractXxxSolver<XxxFunction> That's true: as indicated above, this "intricate" code is CM developer's problem. Applications that use CM should never contain any "AbstractXxx..." types; they should use the interfaces. > If on the other hand someone implements a solver without taking > advantage of the generified BaseAbstractXxxSolver we provide, these > interface simply allow to write: > > AbstractXxxsolver implements XxxSolver > > instead of using only > > AbstractXxxsolver implements BaseUnivariateRealSolver<XxxFunction> This is outside CM's realm. [I don't see why someone would inherit from a CM interface but not use the boiler-plate code in "BaseUnivariateRealSolver<T>".] The purpose of the "BaseUnivariateRealSolver<T>" is to avoid code duplication in CM. It must be general enough so that CM developers should not implement an "AbstractXxxsolver" that would not inherit from it. > I think removing the interfaces from level 1b would simplify the > architecture and help users understand. We would avoid the > losange-shape inheritance between levels 0, 1a/1b and 2. We should probably make it clear that users should not use the generic base class nor the abstract classes, but only the interfaces; maybe adding in the Javadoc a statement like: /** * [...] * * This class is not intended for use outside of the Commons Math library. * @see UnivariateRealSolver * @see PolynomialSolver * @see DifferentiableUnivariateRealSolver */ > The third point is I think I messed thing when I inserted > BracketedUnivariateRealSolver interface back in the hierarchy a few > days ago by extending UnivariateRealSolver. I should probably have > generified it and have it extend BaseUnivariateRealSolver<FUNC > extends UnivariateRealSolver>, thus allowing to have bracketing > solvers also for polynomials and differentiable functions. Do you > agree with this ? Yes. > The fourth point is the generified BaseAbstractUnivariateRealSolver > we provide (level 1b, right of the losange). It forces to implement > a doSolve but in this method we cannot access the function itself > and we cannot reset the evaluations: the fields are private and have > no accessors, even protected, we can only call the function and > incrementing the evaluation at the same time, counting from a > setting the derived class cannot change. It was done on purpose (so as to forbid evaluations that would not increment the counter). > I need access to the > function and I need access to the counter. So i think I will add > some accessors for them. Does this seems reasonable to other > developers ? In refactoring the "solvers" package, I removed a lot of "protected" fields. The current design shows that they were not necessary. Also they are not desirable because they break encapsulation. I'd rather find a way to avoid accessors, and figure out why the new interface does not fit in the design; if we can change it to fit or if we can improve the design so that it fits... > Well, sorry for this long message and the ugly picture. You have a > few hours to read it, as I will not be able to discuss in the few > next hours. > > thanks for your attention ;-) Regards, Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
