Re: [Geotools-devel] First cut of statistics operations (Histogram and Extrema)

[Simone G. <sim...@gm...>]

Ciao Martin,
my fault I started a philosophycal discussion with these thread,
we better stick to the source code if we want to get out of
the discussion in time. I am sure we are both pretty busy these days
and we need to get things done quickly.

Here below you have my thoughts:

<Private vs protected fields in ImagingParameter>
As I said I am fine with using the package private access.


<New protected methods in OperationJAI>
>Some piece of code in the OperationJAI.doOperation(...) method has
been factored
>out, which is a good idea. However I don't think that all of them should b=
e
>given protected access. For example the following variable:
>
>   Boolean requireGeophysicsType
>
>is really an implementation trick; a public API should rather use an
enumeration
>(this work was started with ViewType but not finished).

I agree, but this is the implementation we have right now even if it
may change.
This field could be pretty useful for subclasses which may want to change t=
heir
behaviour or do some preprocessing before doing the actual work. One exampl=
e is
applying operations that may want to expand paletted coverage. Sometimes we=
 can
apply them on the non-geo view some other on the geo view depending on vari=
ous
factors like interpolations and the like. Conclusion is I would leave
it protected.

>
>postProcessResult
>extractSources
>

Same as above.

>   ensureRenderedImage
>   getSubCRS
>   ensureStableDimensions

Go ahead and make them private.

<Adding @deprecated Decorators>
I would not use the deprecation tag until we have  a replacement for
these classes
since I believe is bad to have deprecated classes or interface without prop=
osing
something new. When we'll refactor, we'll deprecate them for the
moment I think the
best thig would be leaving a message that says " be careful we might
refactor this
in the near future".



If you want I can apply these changes myself on trunk and 2.4.x.


Ciao,
Simone.



On 8/31/07, Martin Desruisseaux <mar...@ge...> wrote:
> Simone Giannecchini a =E9crit :
> > However, there is reason behind not using private fields and methods
> > too much and this reason is that we are an open source project. Even
> > though author's  opinion and design goals are important, keeping
> > things as much extensible as possible is far more important to me. So
> > this is why I usually try to keep private filed down to a minimum so
> > that people can extend classes and change behaviour and I don't care
> > id they should not do it, if they do something bad, that's their
> > fault.
>
> I believe that we have different views here. I'm strongly for private acc=
ess to
> internal mechanic (this is encapsulation - usually considered a good prac=
tice in
> Object Oriented programming). I agree with the need to provide facilities=
 for
> user extensions, but through well-though (if possible) API that provide s=
afety
> guards (argument checks, synchronization, etc.), not by direct access to =
the
> internal mechanic that may change at any version.
>
> A "everything public" policy would increase the amount of bugs. Imagine i=
f the
> java.util.HashMap internal mechanic was public; it is a complex mechanic =
and
> erroneous use by developpers would be more than a probability. Everyone w=
ould
> suffer, not just the immediate users, because soon or later I will use th=
e
> library of a third party who exploit HashMap internal mechanic in a boggu=
s way.
> Furthermore, Sun modified HashMap implementation since Java 1.2; it would=
 not
> have been possible if HashMap internal mechanic was public.
>
> It is not just a matter of "use at your own risk". It is also for myself.=
 I need
> to be sure that some internal structures are modified only in a single pl=
ace. If
> you make that internal structure public, I lost this certaincy even with =
my own
> code. This is a road to chaos and unmaintainable code. If you need access=
 to an
> internal structure, please lets investigate why you need that access, if =
it can
> be done by existing public API, and if not which public API could be adde=
d.
>
>
>
> > I have seen that you use a lot of package private access to tie
> > classes together and private methods to hide utilities methods, well
> > this some time is a big limitation because providing design paths for
> > developers is good but building walls to deny them to take some other
> > paths is pretty bad, at least IMHO.
>
> I believe that walls are sometime good if they can prevent developpers to
> venture in unsafe lands - like a barrier in front of a precipice in natur=
al
> parks. If we need to go on the other side, lets see if we should build a =
bridge
> over the precipice, not just remove the barrier, unless the "precipice" i=
s small
> after all. This is a case-by-case analysis, but we should not just remove=
 the
> barrier everytime we meet one.
>
>
> > Could we do something to avoid the usual deprecate-remove cycle for
> > these classes due to their intrinsic experimental nature?
>
> I think that we can just put them in an "unsupported" module for now...
>
>        Martin
>
>


--=20
-------------------------------------------------------
Eng. Simone Giannecchini
President /CEO GeoSolutions S.A.S.
Via Carignoni 51
55041  Camaiore (LU)
Italy

phone: +39 0584983027
fax:      +39 0584983027
mob:    +39 333 8128928


http://www.geo-solutions.it

-------------------------------------------------------