views:

209

answers:

8

Ok, So I have the following situation. I originally had some code like this:

public class MainBoard {
    private BoardType1 bt1;
    private BoardType2 bt2;
    private BoardType3 bt3;
    ...
    private readonly Size boardSize;

    public MainBoard(Size boardSize) {
        this.boardSize = boardSize;

        bt1 = new BoardType1(boardSize);
        bt2 = new BoardType2(boardSize);
        bt3 = new BoardType3(boardSize);
    }
}

Now, I decided to refactor that code so the classes' dependencies are injected, instead:

public class MainBoard {
    private IBoardType1 bt1;
    private IBoardType2 bt2;
    private IBoardType3 bt3;
    ...
    private Size boardSize;

    public MainBoard(Size boardSize, IBoardType1 bt1, IBoardType2 bt2, IBoardType3 bt3) {
        this.bt1 = bt1;
        this.bt2 = bt2;
        this.bt3 = bt3;
    }
}

My question is what to do about Board Size? I mean, in the first case, I just passed the class the desired board size, and it would do everything to create the other kinds of boards with the correct size. In the dependency injection case, that might not be more the case. What do you guys do in this situation? Do you put any kind of checking on the MainBoard's constructor so to make sure that the correct sizes are being passed in? Do you just assume the class' client will be responsible enough to pass the 3 kinds of boards with the same size, so there is no trouble?

Edit

Why am I doing this? Because I need to Unit-Test MainBoard. I need to be able to set the 3 sub-boards in certain states so I can test that my MainBoard is doing what I expect it to.

Thanks

+3  A: 

It is questionable whether Dependency Injection (or Dependency Inversion) should be applied in this case at all. It appears to me that your MainBoard is responsible for managing the lifecycle of the BoardTypes it created in the first sample. If you now do inject your BoardTypes this responsibility must be handled by the consumers of MainBoard.

It is a tradeof between flexibility and additional duties on the consumer side.

On the other hand, if it is sensible to have the lifecycle of BoardTypes be handled externally, it is fine to use Dependency Inversion. Your constructor on MainBoard then needs to ensure it's dependencies are properly statisfied. This would include checking that their Size is equal.

Johannes Rudolph
I am doing this this way so I can easily test the class. Having it like in the first case makes it hard, if not impossible, to test.
devoured elysium
you can still use the second constructor but use a convenience overload or a factory for production.
Johannes Rudolph
A: 

You could get the boardsize from one of Boards that are passed along through DI. This way you could loose the boardSize variable alltogether.

Falle1234
That is true, but strange errors can ocurr if I don't take measures to make sure all the 3 boards have the same size.
devoured elysium
+5  A: 

I would say the boardSize parameter is unnecessary in the second case, but I would add an assertion to ensure that the 3 board sizes are really equal.

But overall, the second case seems dubious to me. I would suggest the first approach, unless you really really need to inject different kinds of boards to the main board. Even so, I would consider using e.g. a board factory instead of passing 3 board parameters to the constructor, e.g.

public interface IBoardBuilderFactory {
    public IBoardType1 createBoardType1(Size boardSize);
    public IBoardType2 createBoardType2(Size boardSize);
    public IBoardType3 createBoardType3(Size boardSize);
}

This would ensure the consistency of the 3 boards regarding both "board family" and size.

We should know more about the context / domain model, specicifcally the relationship of the main board and the child boards in order to decide whether or not DI is the right choice here.

Péter Török
aah, saw your answer after I posted mine... great minds something something...
Java Drinker
Hmmm injecting a factory seems like a good ideia. Let me think about it.
devoured elysium
Hmm the factory is perfect IF the 3 boards either have equal constructors. If they have different constructors I am then bound to having to pass them to the factory's constructor. It can indeed enforce consistency of sizes on the 3 boards but I also get a damn ugly factory constructor :(
devoured elysium
@devoured, not sure if I understand what you mean... the concrete board types as shown in your code sample do have similar constructors, each taking a single `Size` parameter.
Péter Török
Yes, sorry for that. In that case, yes, what you've put is perfect. But I fear later on they might have different constructors each, just that!
devoured elysium
Injecting a factory like that means MainBoard now needs to know constroctor parameters to the individual boards. Better would be to inject a factory with boardSize already set that just returns the values (or see my answer for an alternative).
WW
@WW, factories in the classic sense are stateless. A factory with state sounds more like a [Builder](http://en.wikipedia.org/wiki/Builder_pattern). To me it is entirely reasonable that MainBoard controls the construction parameters of its sibling boards - depending on its functionality, it may need to know the board size anyway. But your suggestion may indeed be better if the main board does not use the board size in other methods. As I noted above, we have too little info to decide.
Péter Török
+1  A: 

What type of info does the BoardTypeX class contain? Does it make sense to have this object injected into your MainBoard. Dependency-Injection, and patterns in general, is not always the solution, and you shouldn't use it just b/c you can. A factory patter of sorts may work better here

public class MainBoard {
    private IBoardType1 bt1;
    private IBoardType2 bt2;
    private IBoardType3 bt3;
    ...
    private Size boardSize;

    public MainBoard(IBoardBuilderFactory factory) {
        this.bt1 = factory.buildBoard(boardSize);
        //...
    }
}

And if the board size should be determined externally, maybe you don't store it at all. Maybe the factory determines what board size to use when it is constructed elsewhere.

Anyway, the point is, design patterns are there to help you accomplish tasks in a clean and maintainable way. They are not hard and fast rules that must be followed

Java Drinker
I am doing it because of Unit-Testing. I need to be able to put the sub-boards in certain states so I can test the MainBoard class.
devoured elysium
Well for testing purposes, you could have a TestBoardBuilderFactory which can return boards in different states from the buildBoard() method. Maybe this class has subclasses, or different constructors or whatever.
Java Drinker
A: 

Hi there.

You could have a BoardTypeFactory that creates BoardTypes like so:

IBoardType bt1 = BoardTypeFactory.Create(boardSize);

Note that there are tons of blog posts and SO answers on how best to write a factory, the above is a simple example.

You can then call

new MainBoard(boardSize, bt1, ....

passing the source size which was used to create the boards.

Cheers. Jas.

Jason Evans
I don't see what I gain in doing it this way. I still have the same problem, not guaranteeing consistency of board sizes when passing them to the MainBoard class.
devoured elysium
Where I said guaranteeing, I meant more something like "enforcing"
devoured elysium
Hi there. Since the same `boardSize` variable was used in the `.Create()` call on the factory and the `new MainBoard()` call, you are using a consistent board size since the exact same value is being passed to the two method calls.
Jason Evans
+1  A: 

-EDIT- Deleted most of my response because others beat me to it :)

Another option - factories. Depending on your requirements, you might find it better (or not) to use factories to solve your problem. There is a good SO discussion here about Factories Vs DI. You could even consider passing a factroy via DI into your class constructor - so your constructor takes a size and a factory class, and defers to the factory class (passing in the size) to get the boards.

Matt Roberts
Looks like most people beat me to it with the factory discussion too ;)
Matt Roberts
+2  A: 

The main advantage of dependecy injection is the insulation from changes to the objects being injected. So in your case the one variable apparent is the size. You would inject the boards into the main board so that the main board no longer needs to know or worry about the size. Also, unless your application needs to maintain 3 distinct behaviors between the different board types I would suggest using a single abstract definition for the board type interface.

public class MainBoard {
    private IBoardType bt1;
    private IBoardType bt2;
    private IBoardType bt3;

    public MainBoard(IBoardType bt1, IBoardType bt2, IBoardType bt3) {
        this.bt1 = bt1;
        this.bt2 = bt2;
        this.bt3 = bt3;
    }
}

It becomes the responsibility of the thing that does the injection (injection framework or assembling code) to ensure these boards are given the proper size. This can be done in a number of ways one example being the main board and injected boards all derive their size from a single external source. Maybe your app, in this case, sizes the injected boards relative to the main board.

So you could have external logic such as:

public class BoardAssembler {
   public static MainBoard assembleBoard(Size size) {
      Size innerBoardSize = deriveSizeOfInternalBoards(size);
      return new MainBoard(new BoardType1(innerBoardSize), new BoardType2(innerBoardSize), new BoardType3(innerBoardSize));
   }
}

Essentially what you're after is the inversion of the construction logic wherever MainBoard is constructed. Start there and extract everything into a factory or some evil singleton factory or static method. Ask yourself, "where is the MainBoard created?" Also ask, "what compoenents and parameters are needed?" Once you've moved all the instantiation logic into a factory it may become simpler to maintain Mainboard and all of its dependencies.

Cliff
A: 

Within the scope of MainBoard, the boardSize is effectively a constant. You only want a single value of it to exist. You need some code like this:

int boardSize = 24;  // Or maybe you get this from reading a file or command line
MainBoardScope mainBoardScope = new mainBoardScope( boardSize );

If you naively made 24 a global or a constant you would have hard-to-see dependencies here because classes would rely on picking up that constant or global statically, rather than through a declared interface (ie. the constructor).

The mainBoardScope holds "singletons" for a group of objects that have the same lifetime. Unlike an old-school singleton, these are not global and are not accessed statically. Then, consider this code that runs when starting up your application (or this scope, in a larger application) to build the object graph:

   MainBoardFactory factory = new MainBoardFactory( mainBoardScope );
   MainBoard board = factory.createMainBoard();

Within that createMainBoard method, you would use the boardSize from the scope to create the three subboards:

   IBoardType1 b1 = injectBoardType1( myScope );
   IBoardType2 b2 = injectBoardType2( myScope );
   IBoardType3 b3 = injectBoardType3( myScope );
   return new MainBoard( scope.getBoardSize, b1, b2, b3 );

Do you need MainBoard to check each of the three boards passed to the constructor are correctly sized? If it is your code creating the boards, then create a unit test for injectMainBoard(). It is not MainBoard's job to ensure it is constructed correctly. It is the factories job to create it, it is the unit test on the factory's job to check it is being done right.

WW