Have you ever come across the problem where, in order to construct the object you are trying to test, you end up creating massive scaffolding code just to pass to the constructor? Perhaps an example is in order:
class ChessGame { // Some instance variables public ChessGame (Player player1, Player player2, int timeLimit) { this.player1 = player1; this.player2 = player2; this.timeLimit = timeLimit; } public void playGame() {...} } class ChessGameTest { @Test public void testChessGamePlaysForXseconds () throws Exception { Player player1 = createMock(Player.class); Player player2 = createMock(Player.class); Handicap handicap1 = new Handicap(10, Rules.Standard); Handicap handicap2 = new Handicap(4, Rules.Standard); expect(player1.getName()).andReturn("D. Duck"); expect(player2.getName()).andReturn("M. Mouse"); expect(player1.getHandicap()).andReturn(handicap1); expect(player2.getHandicap()).andReturn(handicap2); replayAll(); new ChessGame(player1, player2, 10).playGame(); // Assertions .... } }
The problem here is that we’ve specified dependencies on far too many objects and values! By asking for two Player objects, you’re essentially saying “give me two objects which have names, ages, total scores, rankings, biographies, etc… and which also have very detailed Handicap information”.
Ask yourself this question:
- Without looking inside the class file, what exactly does the ChessGame need in order to execute playGame()?
It’s not at all obvious is it! My guess would be that we only really need player names, and handicap information (why do we need the player bio to play a match??). If this is the case, why do we pass all that extra information to the ChessGame constructor, if it’s never going to need it?
I can hear you answering “But it’s far more convenient to package up that information as it exists naturally – inside the Player object. It is more readable, and keeps the number of constructor parameters down!”
But this is not a valid reason because of the following:
- Classes should not ask for more information than they need – otherwise they become difficult and cumbersome to test
- Classes should make absolutely clear what their dependencies are. This is why passing around “Contexts” is a bad thing
We can make this a lot easier by using a Factory Method as a convenience:
class ChessGame { public ChessGame (String player1Name, String player2Name, int player1Handicap, int player2Handicap, int timeLimit) { this.player1Name = player1Name; this.player2Name = player2Name; //..etc } public static ChessGame withPlayers (Player player1, Player player2, int timeLimit) { return new ChessGame ( player1.getName(), player2.getName(), player1.getHandicap().getAbsolute(), player2.getHandicap().getAbsolute()); } public void playGame() {...} } class ChessGameTest { @Test public void testChessGamePlaysForXseconds () throws Exception { new ChessGame("D. Duck", "M. Mouse", 10, 4, 10); // Assertions .... } }
The second ChessGame class accepts only what it needs – it has fewer dependencies on real objects. However, we now have a nice convenience method on the object which takes our existing objects.
Why stop there? Here are some more Factory Methods which you might prove useful:
public static ChessGame fromExistingGame (ChessGame previousGame) {...} public static ChessGame againstBot (BotFactory botFactory) {...} public static ChessGame fromDataFile (File dataFile) {...}
So in conclusion – if you’re finding yourself setting up tonnes of scaffolding for a test, ask yourself whether the unit-to-be-tested is asking too much of you!
Hi,
Another way to handle this would be to let Player implement interface PlayerInfo (just name and handicap) and ChessGame would require two PlayerInfos instead.
Of course, the factory methods can be used for more scenarios, as you mention.
regards,
Vlad
You could also add asserts to the constructor.
That would provide information on what is required to use the Class and would keep the low number of parameters.
Regards
The problem with your approach is that sooner or later you’ll need the mail of the players or anything else and then the constructor has 10 parameters !
The solution is to use PlayerInfo like Vlad says.
Eventually, if you can not have Player implements PlayerInfo, you could have an adapter (PlayerInfoFromPlayer implements PlayerInfo and has a Player attribute)
Just a note: two chess players may have the same name, so in a real context (of a tournament, chess club, or national or world federation), one would want a player ID.
@Franklin – indeed, you are right! This was just supposed to be an illustrative example of a wider point, rather than anything comprehensive. As an odd aside, did you know the Screen Actors Guild doesn’t allow duplicate names? http://answers.yahoo.com/question/index?qid=20080206220412AA4HgTM
@cblin, @vlad – your approach would be an excellent consideration as the number of method arguments increases. I would always start with the “simplest thing that works”, then refactor as I went along. Furthermore, as different methods/constructors require different subsets of information from “Player”, you might end up with a combinatorial explosion of interfaces. However, both approaches are actually alluding to the same thing – that we should only “require” what we need.
@cblin
I see your argument but immidiately throw in anotherone: Single Responsibility Principle. If you need to email is this really the responsibility of the game?
Iwould also start with ctor params of string and int and then only if needed to go to player info interface. With only string and int the game has only a really abstract knowledge and dependency on the player.
Daniel
Vlad’s idea is simpler and tells exactly what is required by the Constructor.
We can add asserts in the code but that again boils down to get into constructor’s details. Which sometimes isn’t possible and shouldn’t be needed. Adding proper javadocs can also be one of the solution but I haven’t seen many programmers updating the javadocs as code evolves.
My husband and i got now relieved when Albert managed to finish up his basic research through your ideas he got from your very own blog. It’s not at all simplistic just to choose to be making a gift of information that some other people may have been trying to sell. We do understand we have the website owner to appreciate because of that. Those illustrations you have made, the easy site navigation, the relationships you will help create – it is mostly exceptional, and it is making our son and our family feel that that matter is interesting, which is certainly truly fundamental. Thanks for all!