Commited on the 2.4 branch as of revision 26799 (will commit to trunk in a few
minutes):

Changes in ImagingParameters
----------------------------
Retrofitted "ImagingParameter[Descriptor]Decorator" functionalities in their
parent class, and deleted the decorators. There is no need for the additional
HashMap provided by the decorators since everything can fit in the existing
structure. Instead of letting the parent classes store unwanted parameters and
patch them in the decorators, we now get the parent classes store the correct
parameters right at begining.

Replaced the Map<String,ParameterDescriptor> constructor argument by
Collection<ParameterDescriptor> because the String key was redundant with the
ParameterDescriptor name. The intend is to reduce the risk of inconsistent user
information.

I have not yet resolved the "ParameterList consistency" issue. However new
implementation make a better effort for fitting the parameter value into the
ParameterList (see ImagingParameters.compatible static method - private of 
course).

Reverted fields to their private state. They have been turned into protected
ones for decorator needs, and the decorators used them wrongly - they put the
'values' field in an inconsistent state compared to the 'asList' one. This is
one reason why encapsulation is important.


Changes in OperationJAI
-----------------------
'extractSources' turned into protected method at your request, after a slight
change in method signature. Moved to the Operation2D superclass, since it is not
really specific to JAI. Because the method is no longer private, some code were
added for performing argument checks.


Miscellaneous remarks
---------------------
I noticed that AbstractOperationJAIDecorator provides a REPLACED_DESCRIPTORS
protected static field. We will have conflict if AbstractStaticticsOperationJAI
is not the only subclass to set it.

The "Abstract" part in the names may not be necessary since we are not
implementing interfaces like "AbstractSet" vs "Set" or "AbstractDatum" vs 
"Datum"...

I noticed in various place code like:

    new HashMap(size);

It is a good practice to specify the required space at collection construction
time if we know it, but in the particular case of HashMap this is not
suffisient. You need to take the load factory in account, which is 0.75 by
default. See HashMap javadoc. The following code is probably closer to what you
intend:

    new HashMap(size + size/4 + 1);



Please let me know if there is issues with this code replacement.

        Martin

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to