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!
