IoC, DI, and a mess...


D

Daniel Pitts

Hmm, I thought I posted this earlier, but Google says otherwise. If
this is a repeat, please respond to the *other* post.


So, I have a RobotFactory, which creates and Robot, all its
components, and then wires the components to the robot. This is my
attempt at using IoC and dependency injection for this.

The wiring is done in one method (shown below). The factory can be
configured once, and then used several times to create new robots that
are the same. The application can have many different RobotFactory
instances.

The problem I have, is that this wiring looks like a mess. I'm trying
to think of ways that could simplify this construct will still giving
me the IoC.

void configureRobot(Robot robot) {
robot.setThrottle(createThrottle());
robot.getThrottle().setRobot(robot);
robot.getHeat().setCoolMultiplier(
chooseDoubleFor("heatsinks", 0.75, 1.00, 1.125, 1.25, 1.33,
1.50));
robot.setArmor(createArmor());
robot.getArmor().setRobot(robot);
robot.setMineLayer(createMineLayer());
robot.getMineLayer().setRobot(robot);
robot.setRadar(createRadar());
robot.setShield(createShield());
robot.getShield().setRobot(robot);
robot.setSonar(createSonar());
robot.getSonar().setRobot(robot);
robot.setTransceiver(createTransceiver(robot));
robot.setTransponder(createTransponder());
robot.setTurret(createTurret());
robot.getTurret().getHeading().setRelation(robot.getHeading());
robot.getTurret().setKeepshift(false);

robot.getTurret().getHeading().setAngle(robot.getHeading().getAngle());

robot.getTurret().setMissileLauncher(createMissileLauncher(robot));

robot.getMineLayer().setArena(robot.getEntrant().getGame().getRound().getArena());
robot.getComputer().setCommQueue(new CommunicationsQueue(
robot.getComputer().getCommQueueMemoryRegion(),

robot.getComputer().getRegisters().getCommunicationQueueHead(),

robot.getComputer().getRegisters().getCommunicationQueueTail()));

robot.getTransceiver().setCommQueue(robot.getComputer().getCommQueue());

robot.getComputer().getCommQueue().setComputerErrorHandler(robot.getComputer().getErrorHandler());

robot.getComputer().getMemory().setErrorHandler(robot.getComputer().getErrorHandler());
robot.getTurret().setScanner(createScanner());
robot.getTurret().getScanner().setRobot(robot);
}

So, as you can see. A big mess. The object graph is a bit of a mess,
but once you've got it wired, it "just works".

Any suggestions for a different approach? Maybe I should create a
context class that knows about all the objects independently of each
other. It doesn't know (or care) how to create them, it gets them
through DI. All it knows about is how to wire them together. That lets
me completely decouple the wiring from the creation. This really
seems like a meta-problem that should be easily solvable.
 
Ad

Advertisements

Z

Zig

Hmm, I thought I posted this earlier, but Google says otherwise. If
this is a repeat, please respond to the *other* post.


So, I have a RobotFactory, which creates and Robot, all its
components, and then wires the components to the robot. This is my
attempt at using IoC and dependency injection for this.

The wiring is done in one method (shown below). The factory can be
configured once, and then used several times to create new robots that
are the same. The application can have many different RobotFactory
instances.

The problem I have, is that this wiring looks like a mess. I'm trying
to think of ways that could simplify this construct will still giving
me the IoC.

void configureRobot(Robot robot) {
robot.setThrottle(createThrottle());
robot.getThrottle().setRobot(robot);

Not sure if this is getting too far off pattern, but I'll throw this out..
If you have methods

Robot.setThrottle
and
Throttle.setRobot

Then it is not obvious to a caller that if one sets one, the same caller
must also set the other. A caller could easily miss those connections. If
you are using setters to set everything up post-construction, I would lean
towards a mechanism like:

public class Robot {
....
private Throttle _throttle=null;
public void setThrottle(Throttle throttle) {
if (throttle==_throttle)
return;
// Decouple the old throttle
if (_throttle!=null)
_throttle.setRobot(null);
// Replace the throttle reference
_throttle=throttle;
// Configure required coupling
if (_throttle!=null)
_throttle.setRobot(this);
}
}

public class Throttle {
....
private Robot _robot=null;
public void setRobot(Robot robot) {
if (robot==_robot)
return;
// Decouple the old robot
if (_robot!=null)
_robot.setThrottle(null);
// Replace the robot reference
_robot=robot;
// Configure required coupling
if (_robot!=null)
_robot.setThrottle(this);
}
}

In this case, the set methods now avoid having object partially set, but
instead completely set the reference to the parameter.

Myself; I like to make objects immutable and fields final by default
unless there is a good reason for them not to be. The downside of this is
it means the constructor has to pick up some of the work. However, that
can still be written as:

public class RobotFactory {
public Robot createRobot() {
return new Robot(this);
}
// package accessible method, but also overridable
protected Throttle createThrottle(Robot robot) {
return new Throttle(robot);
}
}

public class Robot {
private final Throttle _throttle;
//package protected constructor
Robot(RobotFactory factory) {
_throttle=factory.createThrottle(this);
...
}
}

I'm sure someone has come up with this pattern before and named it, but I
don't know off the top of my head. Until I hear the proper name, I tend to
think of this as a "Puppy Pattern", since the objects still need to be
nurtured by their, erhm parent, before they're ready to be released into
the real-world. It is of course the parent's responsibility to make sure
not to feed anything to the pup that it's not yet big enough to handle.

Anyway, that might be so different it's not really DI anymore, but maybe
this was food for thought for you ;)

HTH,

-Zig
 
P

Piotr Kobzda

Daniel said:
Any suggestions for a different approach? Maybe I should create a
context class that knows about all the objects independently of each
other. It doesn't know (or care) how to create them, it gets them
through DI. All it knows about is how to wire them together. That lets
me completely decouple the wiring from the creation. This really
seems like a meta-problem that should be easily solvable.

Try Guice (http://code.google.com/p/google-guice/).


piotr
 
D

Daniel Pitts

Not sure if this is getting too far off pattern, but I'll throw this out.
If you have methods

Robot.setThrottle
and
Throttle.setRobot

Then it is not obvious to a caller that if one sets one, the same caller
must also set the other. A caller could easily miss those connections. If
you are using setters to set everything up post-construction, I would lean
towards a mechanism like:

public class Robot {
...
private Throttle _throttle=null;
public void setThrottle(Throttle throttle) {
if (throttle==_throttle)
return;
// Decouple the old throttle
if (_throttle!=null)
_throttle.setRobot(null);
// Replace the throttle reference
_throttle=throttle;
// Configure required coupling
if (_throttle!=null)
_throttle.setRobot(this);
}

}

public class Throttle {
...
private Robot _robot=null;
public void setRobot(Robot robot) {
if (robot==_robot)
return;
// Decouple the old robot
if (_robot!=null)
_robot.setThrottle(null);
// Replace the robot reference
_robot=robot;
// Configure required coupling
if (_robot!=null)
_robot.setThrottle(this);
}

}

In this case, the set methods now avoid having object partially set, but
instead completely set the reference to the parameter.

Myself; I like to make objects immutable and fields final by default
unless there is a good reason for them not to be. The downside of this is
it means the constructor has to pick up some of the work. However, that
can still be written as:

public class RobotFactory {
public Robot createRobot() {
return new Robot(this);
}
// package accessible method, but also overridable
protected Throttle createThrottle(Robot robot) {
return new Throttle(robot);
}

}

public class Robot {
private final Throttle _throttle;
//package protected constructor
Robot(RobotFactory factory) {
_throttle=factory.createThrottle(this);
...
}

}

I'm sure someone has come up with this pattern before and named it, but I
don't know off the top of my head. Until I hear the proper name, I tend to
think of this as a "Puppy Pattern", since the objects still need to be
nurtured by their, erhm parent, before they're ready to be released into
the real-world. It is of course the parent's responsibility to make sure
not to feed anything to the pup that it's not yet big enough to handle.

Anyway, that might be so different it's not really DI anymore, but maybe
this was food for thought for you ;)

HTH,

-Zig

That is kind of the opposite of DI.
Although, having robot.setThrottle() ensure the invariant of
throttle.setRobot *isn't* a bad thing, but some of the robot
components don't really need to know about the robot (think of it as a
two-way bus versus a one-way). Also, if I design a different type
of throttle that *doesn't* need to know about robot, then I have all
sorts of complications in dealing with that.

So, the approach I decided to try (and it seems clean enough), is to
add a new class called HardwareContext, which has a bunch of setter
methods and fields, one for each component and the robot itself. It
also has a method wireRobot(), which is responsible for connecting all
of the components to each other.

So I guess its a combination of several patterns: Factory pattern with
DI (HardwareSpecification is responsible for creating components and
passing them to HardwareContext) and Configurator pattern
(HardwareContext is responsible for configuring the robot and
components)

I think of it as an assembly line with three people in line. The first
creates the robot shell, the second creates the components, and the
third wires it all together.

It might even be a "cleaner" design if I were to make it have a more
generic event based system, but that's a lot of extra work for this
point. Once I start to expand the combinations of robot/components,
then I'll consider doing that.
 
D

Daniel Pitts

Piotr said:
So, I took a brief look into using Guice, and it looks a bit more
intrusive than what I'd like.

Basically, I'm going to have any number of robots that are differently
configured, and the configuration is depending user interaction.
(Basically, the user writes a definition about the robot. The format for
this definition can *not* be changed)

So, unless I'm missing something, this doesn't seem to be a good fit.
 
A

andrewmcdonagh

So, I took a brief look into using Guice, and it looks a bit more
intrusive than what I'd like.

Basically, I'm going to have any number of robots that are differently
configured, and the configuration is depending user interaction.
(Basically, the user writes a definition about the robot. The format for
this definition can *not* be changed)

So, unless I'm missing something, this doesn't seem to be a good fit.

Sounds like you need to use a DI container... something like
PicoContainer. Its tiny, yet allows you to create objects and their
relationships, using dynamic class loading.

As I'm sure you know, IoC and DI are slightly separate ideas...usually
used together though.

DI has two basic ways of working - Constructor args or setters. These
two means of Injecting the dependency are just about equal in terms of
favourability. Just about all modern Web application/server containers
work this way.

IoC, is about moving the controlling algorithm out of the expected
place and into a new place. Your Robot factory is a usual design
choice for building a robot and its associated abilities, but by using
IoC, you are saying your Robot can build robots, but its instructions
on what to build is moved to a more appropriate and here's the key
'flexible' source of responsibility.

Its this movement of Responsibility which makes IoC so powerful, as it
means our resulting design usually has a better Single Responsibility
Design.

Aside from that, qucikly looking at your snippet of code...I think
some of the issues of messiness is down to there being too finer level
of interactions. By grouping associated interactions into a small set
of Injectors, then your configuration code would be cleaner and easier
to read...

void configureRobot(Robot robot) {
robot.setThrottle(throttleInjector.inject());

robot.getHeat().setCoolMultiplier(
chooseDoubleFor("heatsinks", 0.75, 1.00, 1.125, 1.25, 1.33,
1.50));

// Armor, Mine layer, shield, computer
robot.setDefensiveSystems(defensiveSystemInjector.inject());

// radar, sonar, transceiver/ponder

robot.setEnemyDetectionSystems(enemyDetectionSystemInjector.inject());


robot.setWeaponSystems(weaponSystems.inject());

}

Obviously, using single injectors or composite injectors....

hth

Andrew
 
Ad

Advertisements

P

Piotr Kobzda

Daniel said:
Basically, I'm going to have any number of robots that are differently
configured, and the configuration is depending user interaction.
(Basically, the user writes a definition about the robot. The format for
this definition can *not* be changed)

This kind of configuration seams to be something beyond current release
of Guice. However, there are some workarounds possible:

1) Bind configuration parameters as a named (or, better, "enumed")
constants (see: @Named). This requires a separate injector for each
distinct configuration option set (binding must be performed before
first instantiation), but is probably the best choice in sense of
loosely coupling of a components.

2) Create (or generate, using e.g .'AssistedInject' technique) a
custom factory for a parameterized objects (see "How do I pass a
parameter when creating an object via Guice?" at
http://code.google.com/p/google-guice/wiki/Guice10Recipes).

3) Store configuration before instantiation in e.g. ThreadLocal, and
access it during injection everywhere where needed (for example in
fields' initializers). Unfortunately, it's not easily applicable when
reusing some scoped objects (e.g. bound in Scopes.SINGLETON).

4) Apply additional parameters after instantiation performed by
injector. Appears to be a rational choice in your case.

So, unless I'm missing something, this doesn't seem to be a good fit.

Well, none of the above listed workarounds is perfect. But it seams
that benefits from DI performed by Guice in other cases (circular
dependencies, scopes etc.) are still worth the effort in finding the
best method of applying an extra configuration options.

To measure it somehow, I created a model based on the code fragment
posted by you -- just making your configureRobot() compilable, and
running without errors. After that I added @Inject to each setter and
constructor of the model classes. And after instantiation a robot with:

Robot robot = injector.getInstance(Robot.class);

where injector was made without any module (just a default Guice
bindings); a post-configuration, which I think is similar to your
original one, looks as follows:

void configure(Robot robot) {
robot.getHeat().setCoolMultiplier(
chooseDoubleFor("heatsinks", 0.75, 1.00, 1.125, 1.25, 1.33, 1.50));
robot.getTurret().setKeepshift(false);
}

Other stuff from previous configuration was performed by Guice.

But, of course, the choice is yours. :)


piotr
 
D

Daniel Pitts

Piotr said:
This kind of configuration seams to be something beyond current release
of Guice. However, there are some workarounds possible:

1) Bind configuration parameters as a named (or, better, "enumed")
constants (see: @Named). This requires a separate injector for each
distinct configuration option set (binding must be performed before
first instantiation), but is probably the best choice in sense of
loosely coupling of a components.

2) Create (or generate, using e.g .'AssistedInject' technique) a
custom factory for a parameterized objects (see "How do I pass a
parameter when creating an object via Guice?" at
http://code.google.com/p/google-guice/wiki/Guice10Recipes).

3) Store configuration before instantiation in e.g. ThreadLocal, and
access it during injection everywhere where needed (for example in
fields' initializers). Unfortunately, it's not easily applicable when
reusing some scoped objects (e.g. bound in Scopes.SINGLETON).

4) Apply additional parameters after instantiation performed by
injector. Appears to be a rational choice in your case.



Well, none of the above listed workarounds is perfect. But it seams
that benefits from DI performed by Guice in other cases (circular
dependencies, scopes etc.) are still worth the effort in finding the
best method of applying an extra configuration options.

To measure it somehow, I created a model based on the code fragment
posted by you -- just making your configureRobot() compilable, and
running without errors. After that I added @Inject to each setter and
constructor of the model classes. And after instantiation a robot with:

Robot robot = injector.getInstance(Robot.class);

where injector was made without any module (just a default Guice
bindings); a post-configuration, which I think is similar to your
original one, looks as follows:

void configure(Robot robot) {
robot.getHeat().setCoolMultiplier(
chooseDoubleFor("heatsinks", 0.75, 1.00, 1.125, 1.25, 1.33, 1.50));
robot.getTurret().setKeepshift(false);
}

Other stuff from previous configuration was performed by Guice.

But, of course, the choice is yours. :)


piotr
Okay. There are a few other (choose*For) configurations that were hidden
in my "create*" methods, but I can see the improvement.

So, just to be clear, I'd have to bootstrap it like this:
Guice.createInjector().getInstance(Robot.class);

Now, the real trick is... The Computer has an InterruptTable, PortTable,
and InstructionTable instance, each of which have basically a
Map<Integer, Interrupt>, Map<Integer, Port>, etc... Is there a way with
Guice to set up that mapping? There is quite a bit of complex wiring in
that part of it too.
 
P

Piotr Kobzda

Daniel said:
Now, the real trick is... The Computer has an InterruptTable, PortTable,
and InstructionTable instance, each of which have basically a
Map<Integer, Interrupt>, Map<Integer, Port>, etc... Is there a way with
Guice to set up that mapping? There is quite a bit of complex wiring in
that part of it too.

I think you can achieve this with a binding to a custom provider of each
map type (possibly additionally annotatedWith() in a case of mapping of
the same map types). Now you'll need a Module implementation for at
least a maps mappings, for example:

binder.bind(new TypeLiteral<Map<Integer, Port>>() {})
.toProvider(new Provider<Map<Integer,Port>>() {
@Inject final Provider<Port> portProvider = null;

public Map<Integer, Port> get() {
Map<Integer, Port> map = new HashMap<Integer, Port>();
// initialize a map... e.g.
for(int i = 0; i < 10; ++i) {
map.put(i, portProvider.get());
}
return map;
}
});
// and similar for other maps...


piotr
 
D

Daniel Pitts

Piotr said:
I think you can achieve this with a binding to a custom provider of each
map type (possibly additionally annotatedWith() in a case of mapping of
the same map types). Now you'll need a Module implementation for at
least a maps mappings, for example:

binder.bind(new TypeLiteral<Map<Integer, Port>>() {})
.toProvider(new Provider<Map<Integer,Port>>() {
@Inject final Provider<Port> portProvider = null;

public Map<Integer, Port> get() {
Map<Integer, Port> map = new HashMap<Integer, Port>();
// initialize a map... e.g.
for(int i = 0; i < 10; ++i) {
map.put(i, portProvider.get());
}
return map;
}
});
// and similar for other maps...


piotr
Thanks, Unfortunately, each port/interrupt/instruction has a different
runtime type. eg. there are 47 concrete Instruction implementations,
something like 18 Interrupts, etc... So I currently have code that does:
instruction.put(1, new AddInstruction());
instruction.put(2, new SubtractInstruction());
....
instruction.put(47, new JumpInstruction());


I was thinking of externalizing the mapping to a file (properties or
xml), but I was holding off on that exercise until I needed different
arrangements of instructions.

I suppose I could externalize to a properties file, and then use the
binder you have shown, but that seems a bit messy to me.
 
Ad

Advertisements

A

andrewmcdonagh

Thanks, Unfortunately, each port/interrupt/instruction has a different
runtime type. eg. there are 47 concrete Instruction implementations,
something like 18 Interrupts, etc... So I currently have code that does:
instruction.put(1, new AddInstruction());
instruction.put(2, new SubtractInstruction());
...
instruction.put(47, new JumpInstruction());

I was thinking of externalizing the mapping to a file (properties or
xml), but I was holding off on that exercise until I needed different
arrangements of instructions.

I suppose I could externalize to a properties file, and then use the
binder you have shown, but that seems a bit messy to me.

I would suggest resisting externalising as long as possible. I've yet
to see any particular benefit of describing relationships in Property
files or XML, over just creating a Java class to do so. Indeed I see
a lot of people now wishing they had never externalised into XML and
some projects have even 'un-sprung' (think Spring framework)
applications.

Andrew
 
Ad

Advertisements


Top