[Geoserver-devel] adding a catalog visitor

Hi all,

I have found in a couple of places when writing listeners for the catalog doing stuff like this:

   public void handleModifyEvent(CatalogModifyEvent event) {
     if ( event.getSource() instanceof DataStoreInfo ) {

     }
     else if ( event.getSource() instanceof FeatureTypeInfo ) {

     }
     else if ( ... )

   }

And the list goes on. What would be nice is a visitor for catalog objects. So I could do somethign like this:

   public void handleModifyEvent(CatalogModifyEvent event) {
      CatalogVisitor visitor = new CatlaogVisitgor() {
          void visitDataStoreInfo( DataStoreInfo ) {
          }

          void visitDataStoreInfo( FeatureTypeInfo ) {
          }

          ...
      }

      event.getSource().accept( visitor );
   }

And ideally a CatalogVisitorAdapter which stubs all the visitor methods so that a listener can implement only the methods of relevance.

To do this I think we will probably want to add a root interface for all catalog objects. Say CatalogInfo. It will define the single method:

CatalogInfo {

   void accept( CatalogVisitor );

}

Thoughts? Comments? Objections?

-Justin

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Justin Deoliveira ha scritto:

Hi all,

I have found in a couple of places when writing listeners for the catalog doing stuff like this:

   public void handleModifyEvent(CatalogModifyEvent event) {
     if ( event.getSource() instanceof DataStoreInfo ) {

     }
     else if ( event.getSource() instanceof FeatureTypeInfo ) {

     }
     else if ( ... )

   }

And the list goes on. What would be nice is a visitor for catalog objects. So I could do somethign like this:

   public void handleModifyEvent(CatalogModifyEvent event) {
      CatalogVisitor visitor = new CatlaogVisitgor() {
          void visitDataStoreInfo( DataStoreInfo ) {
          }

          void visitDataStoreInfo( FeatureTypeInfo ) {
          }

          ...
      }

      event.getSource().accept( visitor );
   }

And ideally a CatalogVisitorAdapter which stubs all the visitor methods so that a listener can implement only the methods of relevance.

Works for me, but how do we deal with code that wants to throw a safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

<snip>

Works for me, but how do we deal with code that wants to throw a safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

Good point, never thought of that. However, everything in Catalog is more or less a "closed set" with the exception of StoreInfo and ResourceInfo. And for those if we really wanted we could have the ResourceInfoImpl.accept() and StoreInfoImpl.accept() methods throw the "unknown type" exception.

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Works for me, but how do we deal with code that wants to throw a
safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

I think that shows up as a null pointer exception; or a compile error
- since the visitor pattern demands you strongly type all the options?
You could also make a default adapter that calls a notHandled() method
for each case by default.

public void handleModifyEvent(CatalogModifyEvent event) {

      CatalogVisitor visitor = new CatlaogAdapter() {
          void visitDataStoreInfo( DataStoreInfo ) {
                 // expected
          }
          void notHandled( Object object ){
                 throw new IllegalStateException("Did not expect "+object );
          }

          ...
      }
      event.getSource().accept( visitor );
   }

You can also try and make your visitor stateful to ensure at least one
option was called.
Or make use of that "extraData" convention to communicate the same:

boolean accepted = (boolean ) event.getSource().accept( visitor, false);

Jody

Justin Deoliveira ha scritto:

<snip>

Works for me, but how do we deal with code that wants to throw a safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

Good point, never thought of that. However, everything in Catalog is more or less a "closed set" with the exception of StoreInfo and ResourceInfo. And for those if we really wanted we could have the ResourceInfoImpl.accept() and StoreInfoImpl.accept() methods throw the "unknown type" exception.

Any option that avoids a hunt down in the code for such an obvious
problem is welcomed. Code telling you what you did wrong makes
development and maintenance much easier (too bad contract
based programming did not get mainstream).

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Jody Garnett ha scritto:

Works for me, but how do we deal with code that wants to throw a
safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

I think that shows up as a null pointer exception; or a compile error

Compile error would be best, a NPE please no, it's the worst exception
a program can throw.

- since the visitor pattern demands you strongly type all the options?
You could also make a default adapter that calls a notHandled() method
for each case by default.

Right... this does not seem to play very good in the resource/store
extensibility idea thought. But if we have the visitor work on
ResourceInfo generically we're back at square one, the if/else
that Justin wants to remove is back there... oh my.
On the bright side it's true that with a type specific method each
we get a compiler error (the new kind of resource would not know
what vistor method to call).

Good enough for me, having resource/store really extensible/pluggable
would probably require a ton of other extension points anyways.

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Andrea Aime wrote:

Jody Garnett ha scritto:

Works for me, but how do we deal with code that wants to throw a
safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

I think that shows up as a null pointer exception; or a compile error

Compile error would be best, a NPE please no, it's the worst exception
a program can throw.

Almost the worst. The worst exception a program can throw is an exception that throws NPE in its own toString(), destroying all forensic evidence from the original exception. An example of this is org.geotools.feature.IllegalAttributeException:

http://jira.codehaus.org/browse/GEOT-2111

Pure evil.

http://en.wikipedia.org/wiki/Doctor_Evil

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineer, CSIRO Exploration and Mining
Australian Resources Research Centre
26 Dick Perry Ave, Kensington WA 6151, Australia

Ben Caradoc-Davies ha scritto:

Andrea Aime wrote:

Jody Garnett ha scritto:

Works for me, but how do we deal with code that wants to throw a
safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

I think that shows up as a null pointer exception; or a compile error

Compile error would be best, a NPE please no, it's the worst exception
a program can throw.

Almost the worst. The worst exception a program can throw is an exception that throws NPE in its own toString(), destroying all forensic evidence from the original exception. An example of this is org.geotools.feature.IllegalAttributeException:

http://jira.codehaus.org/browse/GEOT-2111

Hum, there you have a jira issue assigned to no-one, with no one watching it either. A recipe for a monologue with yourself.
I've addded a few watchers that might be interested in commenting
(and eventually solving, thought the best route is always to provide
a patch).

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Ok.. so bringing this to a close. I think arguing about handling extensibility for stores and resources is a bit of a mute point since there would be a lot of other stuff to do if we want to handle this properly. So essentially I argee that we "cross" this bridge when we come to it. And the accept method for StoreInfoImpl and ResourceInfoImpl will throw an exception.

That said and thinking ahead, it should be doable to just add a generic visit method for ResourceInfo and StoreInfo, as well as for the "well known" subtypes we know about. This gives the visitor the freedom to handle ResourceInfo generically, or only choose a specific sub type.

-Justin

Andrea Aime wrote:

Justin Deoliveira ha scritto:

<snip>

Works for me, but how do we deal with code that wants to throw a safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

Good point, never thought of that. However, everything in Catalog is more or less a "closed set" with the exception of StoreInfo and ResourceInfo. And for those if we really wanted we could have the ResourceInfoImpl.accept() and StoreInfoImpl.accept() methods throw the "unknown type" exception.

Any option that avoids a hunt down in the code for such an obvious
problem is welcomed. Code telling you what you did wrong makes
development and maintenance much easier (too bad contract
based programming did not get mainstream).

Cheers
Andrea

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.

Andrea Aime wrote:

Ben Caradoc-Davies ha scritto:

Andrea Aime wrote:

Jody Garnett ha scritto:

Works for me, but how do we deal with code that wants to throw a
safeguard exception in the last else? The usual "there is a programing
error, I don't know what this thing is" kind of exception.

I think that shows up as a null pointer exception; or a compile error

Compile error would be best, a NPE please no, it's the worst exception
a program can throw.

Almost the worst. The worst exception a program can throw is an exception that throws NPE in its own toString(), destroying all forensic evidence from the original exception. An example of this is org.geotools.feature.IllegalAttributeException:

http://jira.codehaus.org/browse/GEOT-2111

Hum, there you have a jira issue assigned to no-one, with no one watching it either. A recipe for a monologue with yourself.

There we have a Jira issue that was assigned to Jody, that Jody unassigned. It was not intended to be a monologue.

I've addded a few watchers that might be interested in commenting
(and eventually solving, thought the best route is always to provide
a patch).

Thanks. This bug bit me hard again today. I do not have a GeoAPI workspace, but I realised that there is a hacky workaround that provides symptomatic relief. I will attach a patch and reassign to Jody (module maintainer).

--
Ben Caradoc-Davies <Ben.Caradoc-Davies@anonymised.com>
Software Engineer, CSIRO Exploration and Mining
Australian Resources Research Centre
26 Dick Perry Ave, Kensington WA 6151, Australia