Followup to Darren’s post about Sam’s posts and my post. If you’re not interested in the detail, skip to the simple rules at the bottom.
Darren says:
GameFactory needs all that stuff is because it is presumably going to pass it down to the Game objects it creates. That is fine, and 5 parameters in a constructor isn’t setting off any code-smell alarms for me. I wouldn’t do it that way however. For a start, I question the need for a factory. Presumably somewhere there is a UI showing a list of possible games waiting for the player to choose one. I see no benefit in having an abstract factory when what I want to do in response to a user clicking ‘Tetris’ is a new TetrisGame(). The indirection of the factory is just getting in the way.If that were true, I’d do it that way too – but most of the time while I’m developing, what I actually want my factory to produce is a MockTetrisGame.
Darren says:
Timer is not what I would do. What the game object really wants is for something else to tell it when time has passed. Something external.With respect to the Timer class I mentioned, I hadn’t intended it to be a java.whatever.Timer, but an interface representing some timing mechanism. Bad naming; sorry. For a Tetris game, this would be the heartbeat which causes the shapes to fall, and is reset by the user moving the shape down. For a PacMan game, this would be the heartbeat which makes the ghosts move. In both cases this would be Mocked before it was ever implemented (at least, it would if you were doing TDD or BDD properly).
Yes; the game could have a TimeDependent interface – but it’s not really the Gamey main class or the GameFactory's responsibility to make the Game listen to the time. The Game responds to the user dropping the shape by asking the timer to reset. The timer, however, sends its events not to the game but to an event thread, so that you don’t get the slightly ugly situation in which a user presses “down” only to find that the timer’s “down” happens first, causing the shape to drop two spaces when you only wanted it to drop by one.
Oh, bother. I forgot to put the event mechanism in the constructor. Now I have to change all those constructors again…
Darren says:
Scorer. The game could expose a public TetrisScore basicScore() method, that a collaborator can call, and do its own post-processing on before showing it to the user.Making the game responsible for calculating the score results in far too much code in the game. It isn’t the game’s responsibility to know that each line completed and removed is worth 10 points. So I created a Scorer class, which can listen to game events and make up its own mind about how many points to give to the user. Now I can decide to weight the score based on the size of the game board, and the Game doesn’t need to know about it.
Behaviour Driven Design is all about responsibilities, and thinking about how something ought to behave. I’ve used games as an example because everyone’s played them, and they’re very easy to visualize, but a little imagination should allow you to see how the complexities of a simple game could multiply for something like, say, a retail till application.
Anyway, here are some simple rules which I try to follow when coding:
- New objects shouldn’t be created outside of the
mainmethod or factories.
You could create a helper class to take care of some of the responsibilities of a class that’s grown too big, it’s true – but how are you going to decide what responsibilities fall to each of the two classes without separate behaviour classes (commonly called “tests”) for each? To truly unit test or design a class, you have to mock out all of its tools. It also allows the tools to be configurable, like theScorerabove. - Tell, don’t ask.
Unless the purpose of the tool is to reveal information (eg: a filereader) it should know how to use the information it’s got. Don’t look at the space between the hammer and the nail and try to judge how far you’ve got left to go for the optimum hammer strike. Just hit the nail with the hammer, and trust that the two of them will work it out somehow.
Mygetphobia is why I don’t like the idea of havinggetmethods on a context or toolbox. I can’t think of a better way of doing it yet, but will keep thinking on it until I do. - Keep your gui and your engine separate.
I shouldn’t have to write that one down. More importantly, though – keep the events in separate threads. Your gui shouldn’t hang just because the back end is busy doing something. Similarly with any system which has a responsive public face and processing behind the scenes. - Try not to add functionality to a class.
If you didn’t put some piece of functionality into a class when you first designed its behaviour, there’s a good chance that it’s not really responsible for the behaviour you’re about to add. Consider providing it with a separate tool to carry out a task, or pushing the functionality down into components which have the information to deal with the problem.
I’ve picked up rules 1, 2 and 3 from elsewhere (1 and 2 quite recently). Will try to add links.
Rule 4 is my own, from bitter-sweet experience.
Since there’s nothing new under the sun you might find these ancient discussions useful:
http://c2.com/cgi/wiki?ContextObject
http://c2.com/cgi/wiki?ContextObjectsAreEvil
Basically a context object hides all your dependencies by coupling everything to one global registry. This means that it’s really easy to make massive sweeping changes but makes it painfully difficult to work out what any one class does since it could conceivably retrieve anything from its context object. Context objects are what happens when people embrace the notion of “loose coupling” without balancing it with the idea of “cohesion.”
The problem that Sony Matthews (the bloke who wrote the article on theserverside that sam newman was dissecting) is trying to solve involves the amount of work needed when you discover that domain object’s dependencies need to be changed. The simpler solution involves centralising the logic needed to create a domain object in a factory. That minimises the changes caused by the discovery of new dependencies.
If you find yourself seeing duplicated signatures containing lots of arguments to constructors it usually indicates a missing abstraction which would bring together all those dependencies. So instead of having:
public FootBallGame(RuleValidator rv, TimeKeeper tk, RuleEnforcer er, WhistleBlower wb, …)
you would have:
public FootBallGame(Referee r)
and the referee would then play all those roles. The end result should be a tree of dependent objects with behaviour (and therefore dependencies) distributed evenly across the levels.
You might also find the PicoContainer anti-patterns page illuminating: http://www.picocontainer.org/Antipatterns
And if you can tolerate examples in C++ then Arthur Riel’s book, Object oriented design heuristics, is full of good advice on this sort of thing: http://bookshelved.org/cgi-bin/wiki.pl?ObjectOrientedDesignHeuristics
–ade
Thanks, Ade. I like the whole Referee thing. That’s kind of what I want to do, but your way is much more meaningful. Hurrah!