Hi guys,
looking at the way we use the managers, we do create a new instance of
each one of them everytime we need to execute some operation.
Right now, we are creating an instance based on some code like :
public static DelAdminMgr createInstance(String contextId) throws
SecurityException
{
if ( Strings.isEmpty( dAdminClassName ) )
{
if ( GlobalIds.IS_REST )
{
dAdminClassName = DelAdminMgrRestImpl.class.getName();
}
else
{
dAdminClassName = DelAdminMgrImpl.class.getName();
}
DelAdminMgr delAdminMgr = (DelAdminMgr)
ClassUtil.createInstance( dAdminClassName );
delAdminMgr.setContextId(contextId);
}
...
There are five problems in this approach :
1) Once we have determinated the type of connector, we can't switch to
any other. This is due to the first test :
if ( Strings.isEmpty( dAdminClassName ) )
As soon as we have defined the class name, it's for ever, unless we set
it to null, which is not really an option.
2) It's static : we base the selection of a class by a gloabbly defined
value, IS_REST. There is no way for the user to switch this to something
different :
public static final boolean IS_REST = ( ( Config.getProperty(
ENABLE_REST ) != null ) && ( Config
.getProperty( ENABLE_REST ).equalsIgnoreCase( "true" ) ) );
3) It's limited. Would we want to add a third possible connector (say,
to a database), then we will have to change all the factory to add
another check like :
if ( GlobalIds.IS_REST )
{
dAdminClassName = DelAdminMgrRestImpl.class.getName();
}
else if ( GlobalIds.IS_DB )
{
dAdminClassName = DelAdminMgrDbImpl.class.getName();
}
else
{
dAdminClassName = DelAdminMgrImpl.class.getName();
}
That requires a new version of the API.
4) It's costly, as we have to call a Class.forName( className
).newInstance() every time we want an instance.
5) It's not context free, as we inject a contextId into the instance.
Now, considering those issues, we can separate them into two different
concerns :
- the first concern is about the static side of such an operation
(points 1 to 4)
- the second concern is about the context 'fulliness' of the approach.
Here, a container approach would be way better. A lightweight container
could do the trick. You require an instance of a class based on a config
parameter which is not static, and you get back an instance. The
container is responsible for the instanciation. Here, the container
would take two parameters : the type of manager, and the type of
connector. Something like :
DelAdminMgr delAdminMgr = container.createInstance( "DelAdminMgr",
context.getConnectionType() );
The connection type is provided by a global context object which is
initialized before we start the API, and that could be modified on the
fly, allowing teh user to require different type of instances.
Adding a new connector is just a mater of defining a mapping between a
name (here "DelAmdinMgr") and a class to instanciate. the tuple <name,
type> would is used to retrieve teh correct class.
A plugin approach, for connectors, would require that they push the
classes to be used into the container. For instance, a DB connector
would register itself into the container by pushing the [
<"DelAmdinMgr", DB_CONNECTOR>, DelAdminDbMgr.class.getName() ] tuple
into the container.
That is one of the possible options, there are many more.
Dealing with the second aspect is a bit more complex, as it requires we
check all the code to be sure that the instance is stateless. Although
it's important to think about doing this check, and eventually move to a
stateless instance, because it will require way less instances to be
created, and we won't have any more to care about the potential memory
cost of it (even if we will have to pass a context to each instance).
In any case, if the instance is stateless, we can stop thinking about
creating a new instance for each call, we just have to return the first
instance we created. And even if they are not stateless, assuming that
the contextID is the only parameter in used, then we could manage a
cache of instance for each specific context.
Thoughts ?
