David Winslow ha scritto: > I have created a GSIP for this improvement. Open the feedback > floodgates! > > http://geoserver.org/display/GEOS/GSIP+35+-+WMS+Decorations
Besides the feedback that other people already provided, wondering, what if a decoration _needs_ certain options. What happens if they are not found? Can we safely assume every decoration will always have fallback values? If not, we need some exception throwing for missing params (a WMS exception is probably ok, it will be turned into a service exception response). Also, I would like Jody to point us at the scalebar decoration code udig is using if possible? Also, very quick code review (did not get into the methods line by line): DecorationLayout: - the code will apparently NPE if the decoration type is not found, line 177 - affinity parsing assumes the user typed the position name verbatim, no extra spaces, no different case... hmmm... not sure it's the best option for a file that's supposed to be edited by a human - is the code actually working fine if the position is not found (and is thus null?). Never tried a switch statement over an enumeration, so not sure how it would react if the enum value is null. decoration package: - no class javadocs? - legend wise, I'd prefer to have the title properly set in GetMapResponse, where the layer sources are available, instead of working around in the legend decorator - some more in code comments would be nice (and yeah, I know there is a lot of GeoServer code without them, but we're trying to improve, right?). There is a nice blog about this that seems to come from "Code complete" : http://www.devtopics.com/13-tips-to-comment-your-code/ (the best one imho is "The code should explain how. The comments should explain why/what." which again comes from "code complete") Cheers Andrea -- Andrea Aime OpenGeo - http://opengeo.org Expert service straight from the developers. ------------------------------------------------------------------------------ _______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
