[chain] Provide a mechanism for encoding catalog and command in a single string

• Key: CHAIN-19
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Unassigned
• Reporter: Joe Germuska
• Affects Version/s: 1.0
• Fix Version/s: 1.1

Environment

Operating System: Mac OS X 10.0
Platform: Macintosh

Description

Considering configuring Struts to look up commands, the prospect of repeatedly
using both a catalog name and a command name is starting to look extremely
tedious. It would be nice to have a single String representation of the tuple,
and it probably makes sense to implement support for this in the base classes of
commons-chain, rather than requiring users to reinvent the wheel.

I have implemented a simple method in the CatalogFactory class which decomposes
a string ID into a catalog name and a command name and returns the command it
looks up. Using ":" as the separator, what precedes the ":" is treated as a
catalog name. If nothing precedes the colon, or if the segment before the colon
is not a valid catalog name, then the entire ID is used as the name of a command
in the "default" catalog.

(Should the "no such catalog" condition be an error instead? Should the
implementation be in CatalogFactoryBase instead of in CatalogFactory? My
thought was that that leaves the syntax a bit slippery.)

Comments

[Joe Germuska]

Created an attachment (id=13858)
Patch to CatalogFactory, CatalogFactoryBaseTestCase implementing single-ID
command lookup.

Just something to get the discussion rolling.

Suggestions on the proposed patch:

• Declare the delimiter as a manifest String constant:

public static final String DELIMITER = ':';

• In getCommand(), treat a String with zero delimiters
  as the name of a command in the default catalog. No
  need to support a syntax that starts with a ':' character,
  unless you happen to define a catalog whose name is a
  zero-length string.

• In getCommand(), treat a String with one delimiter
  as the concatenation of a catalog name and a command
  name, but return null if one or both cannot be found
  (i.e. don't try to change the interpretation to the
  entire string as a command name).

• In getCommand(), treat a String with more than one
  delimiter as an error (reserving that syntax for
  future use), and throw IllegalArgumentException.

[sean schofield]

+1 for Craig's suggestion

Craig, what do you think about my earlier suggestion of using a new class to
do this (CommandUtils) instead of in the CatalogFactory class? Joe commented
that this might be a little bit indirect but I agree with your initial comment
that a getCommand() method in CatalogFactory seems odd.

It seems like a Catalog/CommandUtils class might be appropriate here.
Basically you're looking for a short cut. CatalogFactory's primary reason for
existence is to locate catalogs.

I'm not completely against it, but I think we should try to find a way to
accomplish what Joe wants (which is an excellent idea) in a manner that makes
the most sense. If CatalogFactory is ultimately the best way to do this then
so be it.

[Craig McClanahan]

I'm becoming a little allergic to "FooUtils" class names because they don't
really speak to the purpose. I'd be marginally in favor of CommandFactory or
CommandLookup instead, but don't know if it's worth building a new class for
this single method, and/or whether we think other methods might get added later.
I could live with this method on CatalogFactory, though, as well.

[Joe Germuska]

I particularly don't think this belongs in "Utils" because I think the basic
concept should be integral to usage of Chain. I'm afraid that a "Utils" class
makes it seem second-class.

I'm fine with all of Craig's suggestions, and will upload a revised patch
sometime today. I think we should hold off on inventing any new factories until
we have a better idea of an overall API.

[sean schofield]

I would prefer keeping it in CatalogFactory over creating new factories for
command. If we eventually need a CommandFactory for some other reason, then
maybe we could move the method to it.

[Joe Germuska]

Created an attachment (id=13866)
Updated patch with suggested changes.

This patch implements the changes Craig suggested, and uses commons-logging to
'warn' when the catalog cannot be found.

[Craig McClanahan]

Fixed in nightly build 20041231.