THREDDS Catalog Library

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