Question regarding decoupled Factory

S

Scaramanga

Hello Java Experts!

I'm rather new to designpatterns and I tried to make a Factory of some
kind that can return Commands of various types (create, find, update,
delete) and of different families (different cases).
The code below works but I would like to hear from you experts what you
think of it. CanI go with this design? I specify the Factories and
Commands in .properties files.

T.I.A.

/S

****************************************************************************************

CLIENT

[...]

CommandFactory cf = CommandFactory.getFactory("casetwo");
Command c = cf.getCommand("update");
System.out.println(c.class.getName());

[...]

Gives the output

[...].command.UpdateCaseTwoCommand

And that's what I want; a command of "update" type from the "casetwo"
factory...

****************************************************************************************

public abstract class CommandFactory {
private static Hashtable factories;
Hashtable commands = new Hashtable();

static {
try {
factories = classProperties(CommandFactory.class);
//<-------- ???
}
catch (Exception ex) {
throw new RuntimeException(ex);
}
}

public CommandFactory(Class c) throws FactoryException {
try {
commands = classProperties(c);
}
catch (Exception ex) {
throw new FactoryException(ex);
}
}

public static CommandFactory getFactory(String factory) throws
FactoryException {
return (CommandFactory) factories.get(factory);
}

private static Hashtable classProperties(Class c) throws
ClassNotFoundException, InstantiationException, IllegalAccessException,
IOException {
String classname = c.getName();
String prefix = classname.substring(classname.lastIndexOf(".") +
1);
String suffix = "properties";

Properties propsfile = new Properties();
propsfile.load(c.getResourceAsStream(prefix + "." + suffix));

Hashtable properties = new Hashtable();
Map.Entry entry = null;

for (Iterator i = propsfile.entrySet().iterator(); i.hasNext(); ) {
entry = (Map.Entry) i.next();
properties.put( (String) entry.getKey(),
ObjectCreator.createObject( (String) entry.getValue()));
}

return properties;
}

public abstract Command getCommand(String action) throws
FactoryException;
}


****************************************************************************************

public class CaseOneCommandFactory extends CommandFactory {
public CaseOneCommandFactory() throws FactoryException {
super(CaseOneCommandFactory.class);
//<-------- ???
}

public Command getCommand(String action) throws FactoryException {
return (Command) commands.get(action);
}
}

****************************************************************************************

And the property file for the factory...

"CommandFactory.properties"

caseone=[...].factory.CaseOneCommandFactory
casetwo=[...].factory.CaseTwoCommandFactory
casethree=[...].factory.CaseThreeCommandFactory
casefour=[...].factory.CaseFourCommandFactory

And one command property file for each factory (four of them)

"CaseOneCommandFactory.properties"

create=[...].command.CreateCaseOneCommand
find=[...].command.FindCaseOneCommand
update=[...].command.UpdateCaseOneCommand
delete=[...].command.DeleteCaseOneCommand

"CaseTwoCommandFactory.properties"

create=[...].command.CreateCaseTwoCommand
find=[...].command.FindCaseTwoCommand
update=[...].command.UpdateCaseTwoCommand
delete=[...].command.DeleteCaseTwoCommand

and so on...
 
C

Chris Smith

Scaramanga said:
I'm rather new to designpatterns and I tried to make a Factory of some
kind that can return Commands of various types (create, find, update,
delete) and of different families (different cases).
The code below works but I would like to hear from you experts what you
think of it. CanI go with this design? I specify the Factories and
Commands in .properties files.

This is fine, with the following concerns:

1. This is very generic, and could be confusing if the generic API is
not necessary. For example, if the set of commands is independent of
the "case" and is relatively stable, then if would probably be better
for CommandFactory to have a getUpdateCommand method, instead of relying
on the client to know to pass the String "update" to getCommand.
Similar concerns might apply to getFactory, but most of the time the
choice you made is justified there.

2. The location of your property files is relative to the package of the
CommandFactory class, if I'm understanding propertly. It would be
better to place this in a single documented location, and use a global
resource string to retrieve it.

3. Whenever you leave ANY magic character strings in an implementation
as you've done here, be sure to CLEARLY and UBIQUITOUSLY document what
values are appropriate, or how someone go go about discovering that
information. Nothing is more of a pain than trying to make casual use
of an API and finding out that you need to pass a String that isn't
clearly defined anywhere.

For example, I hate and despise the Java enryption APIs for this reason.
I think whoever designed them should be fired. If you don't want people
to feel that way about you, then the javadoc for your API should
include, very clearly in the documentation for each method that takes a
String, exactly what values are guaranteed to work and what they do, and
--if this is a plugin-based API -- how one might find out about
additional values that will be available for some specific
implementation. Don't let this one slip through the cracks.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation
 
Z

zero

Hello Java Experts!

I'm rather new to designpatterns and I tried to make a Factory of some
kind that can return Commands of various types (create, find, update,
delete) and of different families (different cases).
The code below works but I would like to hear from you experts what
you think of it. CanI go with this design? I specify the Factories and
Commands in .properties files.

T.I.A.

<snip>

I didn't look at the code extensively, but I did notice that CommandFactory
has a public constructor, but is instantiated through a static factory
method. Unless you have a good reason to have that constructor public, why
not make it private?
 
S

Scaramanga

Chris said:
[snip]

3. Whenever you leave ANY magic character strings in an implementation
as you've done here, be sure to CLEARLY and UBIQUITOUSLY document what
values are appropriate, or how someone go go about discovering that
information. Nothing is more of a pain than trying to make casual use
of an API and finding out that you need to pass a String that isn't
clearly defined anywhere.

For example, I hate and despise the Java enryption APIs for this reason.
I think whoever designed them should be fired. If you don't want people
to feel that way about you, then the javadoc for your API should
include, very clearly in the documentation for each method that takes a
String, exactly what values are guaranteed to work and what they do, and
--if this is a plugin-based API -- how one might find out about
additional values that will be available for some specific
implementation. Don't let this one slip through the cracks.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

Hello Chris,

I couldn't agree more when it comes to documentation! You can be *very*
sure that I will document this one in details if I decide to use it!

And thanks for your other input! My factory is very generic, I know, I
like that kind of design! It's kind of beautiful... But it's not really
necessary in this case because nothing going to change that much. I
don't think I can justify the added complexity to the API.

/S
 
R

Roedy Green

My factory is very generic

There are number of problems with overly generic code. I am talking in
general, not about your factory.

1. they are harder to understand.

2. they often don't actually do any useful work. They just LOOK as if
they were doing something.

3. They are more work to maintain than the straight forward hard code
they replace.

4. they become a sort of job insurance, since the work of making
changes gets dumped on the person who created the abstraction.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,766
Messages
2,569,569
Members
45,045
Latest member
DRCM

Latest Threads

Top