I wonder what you think of explaining
org.geotools.data.DataUtilities.defaultValue that
AbstractGridCoverage2DReader and GeneralParameterValue are valid
arguments to that function and hence it should return null instead of
throwing IllegalArgumentException.
Thing is that everytime geoserver renders a coverage two
IllegalArgumentException are thrown per request, comming from:
wms.GetMap.run->geotools.resources.coverage.FeatureUtilities.wrapGridCoverageReader->SimpleFeatureTypeBuilder.add(String, class)->AttributeTypeBuilder.setBinding(Class)->geotools.data.DataUtilities.defaultValue(Class), for arguments:
* org.opengis.parameter.GeneralParameterValue
* org.geotools.coverage.grid.io.AbstractGridCoverage2DReader
It's not like it's really a big deal, and the profiler doesn't really
points to it as a performance problem, but since those two arguments are
so commonly used I think it'd be better for the method to know that than
having thousands of exceptions thrown?
--
Gabriel Roldan
groldan@anonymised.com
Expert service straight from the developers
On Fri, May 6, 2011 at 3:26 PM, Gabriel Roldán <groldan@anonymised.com> wrote:
Hi all,
sorry for the cross-posting.
I wonder what you think of explaining
org.geotools.data.DataUtilities.defaultValue that
AbstractGridCoverage2DReader and GeneralParameterValue are valid
arguments to that function and hence it should return null instead of
throwing IllegalArgumentException.
Thing is that everytime geoserver renders a coverage two
IllegalArgumentException are thrown per request, comming from:
wms.GetMap.run->geotools.resources.coverage.FeatureUtilities.wrapGridCoverageReader->SimpleFeatureTypeBuilder.add(String, class)->AttributeTypeBuilder.setBinding(Class)->geotools.data.DataUtilities.defaultValue(Class), for arguments:
* org.opengis.parameter.GeneralParameterValue
* org.geotools.coverage.grid.io.AbstractGridCoverage2DReader
It's not like it's really a big deal, and the profiler doesn't really
points to it as a performance problem, but since those two arguments are
so commonly used I think it'd be better for the method to know that than
having thousands of exceptions thrown?
I don't have a strong opinion on this one, as you said it does not seem
like a big deal performance wise.
Having DataUtilies.defaultValue() return null might be indeed acceptable,
not sure if there is anything in the code depending on its current
behavior.
Cheers
Andrea
--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
On Fri, 2011-05-06 at 21:00 +0200, Andrea Aime wrote:
On Fri, May 6, 2011 at 3:26 PM, Gabriel Roldán <groldan@anonymised.com> wrote:
> Hi all,
>
> sorry for the cross-posting.
>
> I wonder what you think of explaining
> org.geotools.data.DataUtilities.defaultValue that
> AbstractGridCoverage2DReader and GeneralParameterValue are valid
> arguments to that function and hence it should return null instead of
> throwing IllegalArgumentException.
>
> Thing is that everytime geoserver renders a coverage two
> IllegalArgumentException are thrown per request, comming from:
>
> wms.GetMap.run->geotools.resources.coverage.FeatureUtilities.wrapGridCoverageReader->SimpleFeatureTypeBuilder.add(String, class)->AttributeTypeBuilder.setBinding(Class)->geotools.data.DataUtilities.defaultValue(Class), for arguments:
> * org.opengis.parameter.GeneralParameterValue
> * org.geotools.coverage.grid.io.AbstractGridCoverage2DReader
>
> It's not like it's really a big deal, and the profiler doesn't really
> points to it as a performance problem, but since those two arguments are
> so commonly used I think it'd be better for the method to know that than
> having thousands of exceptions thrown?
I don't have a strong opinion on this one, as you said it does not seem
like a big deal performance wise.
Having DataUtilies.defaultValue() return null might be indeed acceptable,
not sure if there is anything in the code depending on its current
behavior.
I'm not proposing to change the behaviour of DataUtilities.defaultValue
(which is throwning a IAE upon an unknown type), but to just make it
know those are two possible values for which the default value is null?
Nonetheless, just wondering. Not a big deal.
Cheers
Andrea
--
Gabriel Roldan
groldan@anonymised.com
Expert service straight from the developers
That sounds fine; in that we should allow for expected behaviour.
How do you feel about removing .wrapGridCoverageReader? We only did this to work around issues in the origional MapLayer (that was only willing to store features).
If you update the renderer to use GridReaderLayer I think you will avoid this problem completely? (GridReaderlayer uses wrapGridCoverageReader in the toFeatureCollection method in order to preserve that status quo).
–
Jody Garnett
On Friday, 6 May 2011 at 11:26 PM, Gabriel Roldán wrote:
Hi all,
sorry for the cross-posting.
I wonder what you think of explaining
org.geotools.data.DataUtilities.defaultValue that
AbstractGridCoverage2DReader and GeneralParameterValue are valid
arguments to that function and hence it should return null instead of
throwing IllegalArgumentException.
Thing is that everytime geoserver renders a coverage two
IllegalArgumentException are thrown per request, comming from:
wms.GetMap.run->geotools.resources.coverage.FeatureUtilities.wrapGridCoverageReader->SimpleFeatureTypeBuilder.add(String, class)->AttributeTypeBuilder.setBinding(Class)->geotools.data.DataUtilities.defaultValue(Class), for arguments:
It’'s not like it’s really a big deal, and the profiler doesn’t really
points to it as a performance problem, but since those two arguments are
so commonly used I think it’d be better for the method to know that than
having thousands of exceptions thrown?
WhatsUp Gold - Download Free Network Management Software
The most intuitive, comprehensive, and cost-effective network
management toolset available today. Delivers lowest initial
acquisition cost and overall TCO of any competing solution. http://p.sf.net/sfu/whatsupgold-sd
On Sat, May 7, 2011 at 3:32 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:
That sounds fine; in that we should allow for expected behaviour.
How do you feel about removing .wrapGridCoverageReader? We only did this to
work around issues in the origional MapLayer (that was only willing to store
features).
If you update the renderer to use GridReaderLayer I think you will avoid
this problem completely? (GridReaderlayer uses wrapGridCoverageReader in the
toFeatureCollection method in order to preserve that status quo).
Sounds like a good cleanup, though one that will keep up us handling regressions
in the rendering subsystem for a while. In any case, this is the start
of the trunk
so the best time to do this kind of change.
It's also related to the other thread. Is there a proposal? Does the
proposal have
resourcing to back it?
Cheers
Andrea
--
-------------------------------------------------------
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
That sounds fine; in that we should allow for expected behaviour.
How do you feel about removing .wrapGridCoverageReader? We only did this to
work around issues in the origional MapLayer (that was only willing to store
features).
If you update the renderer to use GridReaderLayer I think you will avoid
this problem completely? (GridReaderlayer uses wrapGridCoverageReader in the
toFeatureCollection method in order to preserve that status quo).
Sounds like a good cleanup, though one that will keep up us handling regressions
in the rendering subsystem for a while. In any case, this is the start
of the trunk
so the best time to do this kind of change.
It’s also related to the other thread. Is there a proposal? Does the
proposal have
resourcing to back it?
Cheers
Andrea
–
Ing. Andrea Aime
GeoSolutions S.A.S.
Tech lead
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy