Hi Jody, I see your point about StyleVisitor implementations not covered by Abstract StyleVisitors which may break. However, as far as I can see from Ian changes on his branch, he has already made several visitors extending the AbstractStyleVisitor which implements the new method so that change should be transparent. (Ian, could you check if all StyleVisitor implementations are properly updated?).
I wonder if that "-1" should be revisited as "+0" provided these details. I'm a bit against the idea of having parameter names in the ContrastEnhancement Interface. This will require that whenever a new algorithm is defined with new parameters, we have to define them in that interface which in my opinion it shouldn't contain such kind of information (again... that's only my opinion). They should be part of the specific ContrastMethod implementation as in this example from Ian's code which check the parameter names: https://github.com/ianturton/geotools/blob/contrast/modules/library/main/src/main/java/org/geotools/styling/Exponential.java Same approach can be used for algorithms. Please, let us know what do you think about our feedbacks since we have to complete this work before the end of the week (freeze is incoming). . Daniele On Tue, Aug 4, 2015 at 11:50 PM, Jody Garnett <[email protected]> wrote: > Sorry Ian, getting muddle between your proposal text and the subsequent > API change section. > > Reviewing both together, you are not just talking about changing from > CodeList values to Strings, you are talking about changing ContrastMethod > to a class ... with a type and some of those types need parameters so you > have a couple of expressions. > > Rather than go turtles all the way down (since ContrastMethod was > originally a String of just this nature for ContrastEnhancement) can we > take this back up a notch. I ask this in part due to the change to the > StyleVisitor required by your proposal. We have a lot of style visitors not > all of which are covered by abstract classes. > > Going to revise my vote to -1 due to the change to StyleVisitor being too > invasive. > > Here is an alternative for discussion: > > public interface ContrastEnhancement { > > public static String GAMMA = "GAMMA"; > public static String MIN_VALUE = "minValue"; // used for ConstrastMethod. > NORMALIZE > public static String MAX_VALUE = "maxValue"; // used for ConstrastMethod. > NORMALIZE > public static String NORMALIZATION_FACTOR"= "normalizationFactor"; > public static String CORRETION_FACTOR = "correctionFactor"; > > public ContrastMethod getMethod(); // Literal value of > getConstrastMethod() or NONE > public Expression getConstractMethod(); > public Expression getGammaValue(); // short cut for > getParameters().get("GAMMA") > public Expression getAlgorithm(); // one of StretchToMinimumMaximum, > ClipToMinimumMaximum, ClipToZeroMaximum > public Map<String, Expression> getParameters(); > } > > Where ContrastMethod remains a code list: > > public final class ContrastMethod extends CodeList<ContrastMethod> { > public static final ContrastMethod NORMALIZE = new > ContrastMethod("NORMALIZE"); > public static final ContrastMethod HISTOGRAM = new > ContrastMethod("HISTOGRAM"); > public static final ContrastMethod NONE = new ContrastMethod("NONE"); > public static final ContrastMethod LOGARITHMIC = new > ContrastMethod("LOGARITHMIC"); > public static final ContrastMethod EXPONENTIAL = new > ContrastMethod("EXPONENTIAL"); > } > > Writing this out the result is much less invasive, I think you could > introduce ContrastMethod.NORMALIZE (this is stretch to min / > max), ContrastMethod.NORMALIZE_CLIP (this is clip to min max), > ContrastMethod.NORMALIZE_CLIP_FROM_ZERO (this is eclipse to zero max) and > avoid the getAlgorithm() method completely with no loss of expressive power. > > Please consider the above suggestion, it is a shame you were not in the > meeting today - let me know if you would like a quick Skype chat to burn > through this. > > > ------------------------------------------------------------------------------ > > _______________________________________________ > GeoTools-Devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > -- == GeoServer Professional Services from the experts! Visit http://goo.gl/it488V for more information. == Ing. Daniele Romagnoli Senior Software Engineer GeoSolutions S.A.S. Via Poggio alle Viti 1187 55054 Massarosa (LU) Italy phone: +39 0584 962313 fax: +39 0584 1660272 http://www.geo-solutions.it http://twitter.com/geosolutions_it ------------------------------------------------------- *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003* Le informazioni contenute in questo messaggio di posta elettronica e/o nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il loro utilizzo è consentito esclusivamente al destinatario del messaggio, per le finalità indicate nel messaggio stesso. Qualora riceviate questo messaggio senza esserne il destinatario, Vi preghiamo cortesemente di darcene notizia via e-mail e di procedere alla distruzione del messaggio stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od utilizzarlo per finalità diverse, costituisce comportamento contrario ai principi dettati dal D.Lgs. 196/2003. The information in this message and/or attachments, is intended solely for the attention and use of the named addressee(s) and may be confidential or proprietary in nature or covered by the provisions of privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection Code).Any use not in accord with its purpose, any disclosure, reproduction, copying, distribution, or either dissemination, either whole or partial, is strictly forbidden except previous formal approval of the named addressee(s). If you are not the intended recipient, please contact immediately the sender by telephone, fax or e-mail and delete the information in this message that has been received in error. The sender does not give any warranty or accept liability as the content, accuracy or completeness of sent messages and accepts no responsibility for changes made after they were sent or for other risks which arise as a result of e-mail transmission, viruses, etc.
------------------------------------------------------------------------------
_______________________________________________ GeoTools-Devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
