[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: THREDDS Catalog Library



Travis Stevens wrote:

Hello,

I have been looking at the java implementation of the catalog library found at http://my.unidata.ucar.edu/content/projects/THREDDS/tech/ClientLibraries.html and I would like to offer some constructive criticism of the design.

It seems to me that the goal of this library is to allow developers to create InvCatalog objects which can be rendered as THREDDS catalog XML files. I believe that certain changes to the interface could make implementing the API a bit easier (feel free to sing along http://www.unidata.ucar.edu/projects/THREDDS/tech/javadoc/v2.0/catalog/index.html).

The main package is thredds.catalog. Unfortunately, this package contains multiple abstractions -- the base API and some implementations of the base API, conversion APIs and creational tool. The base API consists of these classes:

CollectionType
DataFormatType
DateType
InvAccess
InvCatalogRef
InvCatalog
InvDataset
InvDocumentation
InvService
ServiceType
MetadataType
ThreddsMetadata

I have chosen these because they are the only classes that are directly related to the Dataset Inventory Catalog Specification http://my.unidata.ucar.edu/content/projects/THREDDS/tech/catalog/InvCatalogSpec.htm . These classes should be in the thredds.catalog package by themselves. I would suggest these classes be interfaces and not abstract classes. Remember that in Java, one can not extend more than one class, but they can implement more than one interface. This is really the difference between being an object (abstract class) and having a particular behavior (interfaces). What you really want is for an object to have InvCatalog behavior not to be an InvCatalog.

I would suggest creating a package which contains abstract implementations of the thredds.catalog API. Maybe a thredds.catalog.impl package. In this package you would put those convenient abstract classes.

InvAccessImpl
InvCatalogImpl
InvDatasetImpl
InvDatasetProxyImpl

I would suggest creating a package for the conversion API maybe thredds.catalog.conversion.
InvCatalogConverterIF
MetadataConverterIF

...and now that these converters are in an API based package, there is no need to add "IF" to the end of the name. I find it clearer to say "Object o has Metadata Converter behavior" instead of "Object o has Metadata Converter Interface behavior".

The JaxpFactory seems more of a tool, maybe it belongs in thredds.catalog.tool and InvCatalogFactory is a creational object that might belong in thredds.catalog.factory.

Thinking in terms of each abstraction would alleviate method signatures like "thredds.catalogInvDataset.findDatasetByName(String name): InvDatasetImp". Having an abstract object return a concrete object limits what one can do when implementing the API.

Anyway, thats just my $0.02.


-Trav




Travis, thanks for your thoughts on this.

I dont particularly like the abstract/impl dichotomy either. The previous design used interfacces, but that has tradeoffs also. Package placement and finding the right balance of abstraction and detail hiding is a black art.

When I can, I will think harder about the details of your message and try to say something more intelligent.

thanks again!