Nick Brettel's eight puzzle design

From CSSEMediaWiki
(Difference between revisions)
Jump to: navigation, search
 
(8 intermediate revisions by one user not shown)
Line 81: Line 81:
  
  
[[Tell, don't ask and [[Law of Demeter]] are followed - the ''Participant'' wraps up calls to the role.  To do this [[Recursion introduction]] is used: the participant just relays calls to ''printData()'' and ''addName()'' to methods in one of its roles (of the same name), depending on the ''currPhase''.  In this situation I think applying these maxims is definitely beneficial.  The ''ParticipantRole'' becomes transparent so we don't even need to know that a participant has a role, as far as anyone using a ''Participant'' is concerned, it can ''addName()'' and ''printData()'', and we don't care how.  It is effectively InformationHiding.  The alternative, something like ''participant.getCurrentRole().addName()'', requires the client to know that the participant has a role, it has a getter to access it, it stores the names, and we can change them using the method ''addName()''...
+
[[Tell, don't ask]] and [[Law of Demeter]] are followed - the ''Participant'' wraps up calls to the role.  To do this [[Recursion introduction]] is used: the participant just relays calls to ''printData()'' and ''addName()'' to methods in one of its roles (of the same name), depending on the ''currPhase''.  In this situation I think applying these maxims is definitely beneficial.  The ''ParticipantRole'' becomes transparent so we don't even need to know that a participant has a role, as far as anyone using a ''Participant'' is concerned, it can ''addName()'' and ''printData()'', and we don't care how.  It is effectively InformationHiding.  The alternative, something like ''participant.getCurrentRole().addName()'', requires the client to know that the participant has a role, it has a getter to access it, it stores the names, and we can change them using the method ''addName()''...
  
 
So what is this ''addName()'' method?  I needed some way to set a participants name.  However, to set a pair's names, we would need two names, as opposed to the one required to set an individual's name.  But if ''IndividualRole'' and ''PairRole'' have different ''setName()'' methods (with a different number of arguments), then a ''Participant'' could only ''setName()'' if it knew whether its role is training or testing.  A ''Participant'' shouldn't need to know the types of its roles, just that it has roles, and what their interface is.  Simply put, I want to [[Program to the interface not the implementation]].  To do this, a uniform way to set name(s) is required, even though we don't know how many names.  I choose to do this by using a common interface: an ''addName()'' method sets one name, and it can just be called a couple of times to set a couple of names (for pairs).  I noticed later that this was a similar idea to the [[Builder pattern]] (I don't think I can go so far as to say this is an application of the pattern) - we add parts (names) until we have built a product, and then we normally get the product (in my case, we change the product by adding parts, so there is no need to get it - we already had it).  An alternative approach would have been to pass in a Collection of names.  However, I decided this was not as good, as the number of items in the collection would be very small (and a majority of the time participants are individual), so there would be a lot of unnecessary Collections, and they do seem like overkill.
 
So what is this ''addName()'' method?  I needed some way to set a participants name.  However, to set a pair's names, we would need two names, as opposed to the one required to set an individual's name.  But if ''IndividualRole'' and ''PairRole'' have different ''setName()'' methods (with a different number of arguments), then a ''Participant'' could only ''setName()'' if it knew whether its role is training or testing.  A ''Participant'' shouldn't need to know the types of its roles, just that it has roles, and what their interface is.  Simply put, I want to [[Program to the interface not the implementation]].  To do this, a uniform way to set name(s) is required, even though we don't know how many names.  I choose to do this by using a common interface: an ''addName()'' method sets one name, and it can just be called a couple of times to set a couple of names (for pairs).  I noticed later that this was a similar idea to the [[Builder pattern]] (I don't think I can go so far as to say this is an application of the pattern) - we add parts (names) until we have built a product, and then we normally get the product (in my case, we change the product by adding parts, so there is no need to get it - we already had it).  An alternative approach would have been to pass in a Collection of names.  However, I decided this was not as good, as the number of items in the collection would be very small (and a majority of the time participants are individual), so there would be a lot of unnecessary Collections, and they do seem like overkill.
 +
 +
=== Slide Show ===
 +
 +
The experiment consists of pages of instruction, and the task to perform.  Displaying these instructions, and the task itself, is analagous to a slide show.  We have a number of slides (or "cards", thinking like Java's ''CardLayout'') and for each, we show it, then move on.  A key abstraction that was identified in the refactoring process, was that all panels that are displayed to the user abide by the same interface.  Namely, all my panels follow a simple process - they show a card, until they are done.  I decided a ''Card'' abstract class was necessary that captured this common behaviour.
 +
 +
The entire experiment application is controlled by the ''ExperimentGUI''.  This is also the main ''JFrame'' that contains any ''JPanel'' that is displayed.  It keeps track of all the instruction ''Card''s that can be displayed, plus the ''EightPuzzlePanel'' card that contains the gui by which someone solves the puzzle.
 +
 +
[[image:card.gif]]
 +
 +
As you can see above, by providing a ''Card'' abstraction, the coupling is less tight - the ''ExperimentGUI'' depends on the abstraction rather than the specific panels.
 +
The abstract ''Card'' also gives us a uniform way to tell the ''ExperimentGUI'' to move to the next slide.  The ''Card'' stores the ''ExperimentGUI'' and uses it in the (concrete) ''done()'' method.  This method just calls the ''panelDone()'' method in ''ExperimentGUI''.
 +
 +
What the instruction ''Card''s are themselves is not very important.  ''ExplanationPanel'' is just a generic panel, with a text box and a next button, for displaying any textual instructions to the user.  The abstract ''StartPanel'' and ''StartTestingPanel'' classes are also shown.  These, much as the name suggests, are panels for instructions when the experiment begins, and just before testing, respectively.  We'll look at ''StartPanel'' a bit more later on.
 +
 +
To create a new ''Card'', all that is needed is to provide implementations for the abstract method ''show()'' (for any behaviour when the ''Card'' is first displayed) and call the ''done()'' method when finished.
 +
 +
 +
==== Dependency Inversion Principle, applied ====
 +
 +
How I ended up with this design deserves some explanation.  You can see what I had prior to refactoring at [[#Old Design]].  I was concerned with the three two-way dependencies - each panel was tightly coupled to the ''ExperimentGUI''.  Firstly I considered using the [[Observer pattern]].  I said at the time:
 +
:'...I could make each "card" observable, to decrease the coupling between ExperimentGUI and a card. This doesn't fit the ObserverPattern that well: there is only one object doing the observing, and many being observed. However, it would get rid of an owner reference. Is it really worth it when there's only one Observer, ''ExperimentGUI''? I think so. [[Observer pattern]] may make things a bit more complicated, but that's better than having the unnecesary owner reference: a card doesn't need to know its owner, only how to say "I'm done".'
 +
 +
Then I realised, there had to be a better way.  Strike one.
 +
 +
My next thoughts were that I could use the [[Mediator pattern]].  I thought ''the interactions between the GUI and the Panels is well-defined (e.g. GUI says "show this panel", a panel says "I'm done"). But then, is the State really a state of the gui? Or a state of the Mediator? ''  Upon further examination, I realised that the [[Mediator pattern]] also didn't really fit.  Strike 2.  The interactions between objects are well-defined, but not very complex.  Applying mediator in its entirity would be overkill to the extreme, but looking at how the [[Mediator pattern]] was a good idea helped me to see the solution.
 +
 +
Finally, I realised, the ''ExperimentGUI'' was depending on the low-level details of each ''Card''. What I needed, was to listen to [[Bob Martin]] and follow the [[Dependency inversion principle]].  He states that what is needed is a dependency inversion, so in my case, the ''ExperimentGUI'' would no longer depend on the panels, but on the ''Card'' abstraction, and the panels would also depend on this ''Card'' abstraction.  Thus, the dependency is inverted.  This gives us our design above!
 +
 +
==== ExperimentGUI still depends on EightPuzzlePanel :-/ ====
 +
 +
You can see, however, that there is still one dependency to a subclass of the ''Card'' hierarchy - the ''ExperimentGUI'' must have an ''EightPuzzlePanel''.  This is due to the fact the ''EightPuzzlePanel'' is in charge of looking after the ''Participant''.  You could move the ''Participant'' to the ''ExperimentGUI'' controller class - but I think it makes more logical sense where it is.  The ''EightPuzzlePanel'' is the only thing that needs to know about the participant - as they do the experiment (by using the panel) data about the participant is recorded.
 +
Here again we have an object (''ExperimentGUI'') depending on the low-level details (''EightPuzzlePanel''), which doesn't fit with the [[Dependency inversion principle]].  This would not be too difficult to fix.  We need to represent the fact that we require some ''TaskCard'' (if you like) that is a card that represents the task to be completed by the ''Participant''(s).  A ''TaskCard'' would require a ''startTesting()'' method and a ''addName('' method.
 +
I considered doing this, but didn't for two reasons.  Firstly, it really isn't necessary yet.  [[Do the simplest thing that could possibly work]], [[Keep it simple]] and [[You ain't gonna need it]] were begging me not to.  It would make more sense only to do it once a different puzzle was actually required to be plugged in, instead of the eight-puzzle.  Secondly, I just ran out of time :)  But hopefully you can see it would be easy to refactor, when necessary, and the goal of making the experiment extensible would still be possible.
 +
 +
=== Rigid Procedure ===
 +
 +
The process of participating in my experiment followed a rigid process.  First you were presented with some starting instructions.  Then you would go into a training phase.  In the training phase you would, n times repeatedly, be presented with a "get ready screen" then the eight-puzzle itself.  And the process continues.  You can see that there are various stages of the experiment, and you step through them, so they could be represented as states.  I decided to use the [[State pattern]] for modelling this.
 +
 +
[[image:state.gif]]
 +
 +
The merit of using the [[State pattern]] here is dubious.  I have often weighed up whether, in this case, it is beneficial, and I've decided that marginally, yes, with some costs.  The first thing to consider, is what behaviour is changing as you go through the states.  Basically, the only thing is what ''Card'' is shown, and what state you go to next.
 +
 +
Using a state pattern definitely does get around a big ugly method containing many if-elses, and we should [[Beware value switches]].  Also, we can store additional information about each state (e.g. iteration) in the state class itself, following [[Keep behaviour with data]].  And, as the [[Gang of four]] say, ''it localises state-specific behaviour and partitions behaviour for different states''.  However, it also results in distributing the logic, relating to the one function, across many classes.  As a result, the design is a lot less compact (as Martin Grenfell said, ''it's fat'').  Adding new states may make maintenance difficult.  Also, ''ExperimentGUI'' coordinates all the functionality (the states just call methods on it, using an owner reference), so ''ExperimentGUI'' becomes bigger.
 +
 +
Weighing up all these pros and cons, I decided to use it.
 +
 +
=== Varied Instructions ===
 +
 +
When users start the experiment, they are presented with instructions.  However, the instructions vary slightly for the individual or collaborative groups.  For example, both groups get a welcome message, but the collaborative group needs a specific message saying how they will be working collaboratively.  To achieve this, I used the [[Template method pattern]].
 +
 +
[[image:instructions.gif]]
 +
 +
The idea behind using this pattern was to create a general structure for the instructions, but allowing them to vary.  Again this allows us to follow the [[Open closed principle]].  We are open to extend ''StartPanel'', but we can only extend specific aspects of the instructions (namely the "enter name" message and the "process" message).  We are closed to modifying any other instructions, the order of the instructions, or what the "welcome" and "process" messages are.
 +
 +
Again the question arises, is it really worth it?  We could have an arguably simpler design where instead we have, say, a ''setInstructions()'' method and some constant Strings to make use of,
 +
e.g.
 +
 +
    public static const String WELCOME_STRING="Welcome to my experiment\n";
 +
 +
 +
Instructions could then be set by (something like):
 +
 +
    setInstructions(WELCOME_STRING + "bla bla bla\n" + PROCESS_MESSAGE_STRING);
 +
 +
Using a [[Template method pattern]] is a lot more rigid - the order of instructions is clearly defined, and the common behaviour is factored out, so all ''StartPanel''s (ever) must have the fundamental welcome and process messages.  In fact, the welcome and process messages are made private to ensure to subclasses can't override them.
 +
 +
== The Big Picture ==
 +
 +
Here you can view the entire design, in all its glory.  Note that the title was meant literally - the design is a very big picture size-wise.  [[#Tour Of The Design]] gives a walk-through of the design if it doesn't seem to make sense.  This diaagram is just to get an idea of where everything fits in.  Also, this design is often referred to, so feel free to print it off :).
 +
 +
[[image:bigPicture.jpg]]
 +
 +
== Design Patterns ==
 +
 +
The following design patterns are present in my design.
 +
 +
=== Template Method Pattern ===
 +
 +
A [[Template method pattern]] is used to vary instructions depending on the participant's collaboration condition, but still following a uniform structure.  It is discussed more at [[#Varied Instructions]].
 +
 +
'''Participants: '''
 +
 +
{|
 +
|-
 +
| ''AbstractClass'' || ''StartPanel''
 +
|-
 +
| concrete ''templateMethod()'' || ''getInstructions()''
 +
|-
 +
| abstract ''primitiveOperation1()'' || ''getEnterNameMessage()''
 +
|-
 +
| abstract ''primitiveOperation2()'' || ''getConditionMessage()''
 +
|-
 +
| ''ConcreteClass1'' || ''IndividualStartPanel''
 +
|-
 +
| ''ConcreteClass2'' || ''CollaborativeStartPanel''
 +
|-
 +
|| ''ConcreteClass3'' || ''CollaborativeTestingStartPanel''
 +
|-
 +
|}
 +
 +
=== State Pattern Number 1 ===
 +
 +
A ''Participant'' has a ''ParticipantRole'' that follows the [[State pattern]].  This allows participants to be individuals or pairs, behave differently depending on which, and change from one to the other.  This could easily be considered a [[Strategy pattern]] however - this and related discussion is at [[#Logging Participants]].
 +
 +
'''Participants: '''
 +
{|
 +
|-
 +
| ''Context'' || ''Participant''
 +
|-
 +
| ''State'' || ''ParticipantRole''
 +
|-
 +
| abstract ''handle()'' || ''printData()''
 +
|-
 +
| ''ConcreteState1'' subclass || ''IndividualRole''
 +
|-
 +
| ''ConcreteState2'' subclass || ''PairRole''
 +
|-
 +
|}
 +
 +
''addName()'' could be considered a second ''handle()'' method, but I won't describe it here.  I think it is more like a [[Builder pattern]] below...
 +
 +
=== State Pattern Number 2 ===
 +
 +
A [[State pattern]] is also used for the various stages of the experiment.
 +
 +
'''Participants: '''
 +
{|
 +
|-
 +
| ''Context'' || ''ExperimentGUI''
 +
|-
 +
| ''State'' || ''ExperimentState''
 +
|-
 +
| abstract ''handle()'' || ''showNextCard()''
 +
|-
 +
| ''ConcreteState1'' subclass || ''TrainingInstructionsState''
 +
|-
 +
| ''ConcreteState2'' subclass || ''GetReadyState''
 +
|-
 +
| ''ConcreteState3'' subclass || ''FinishState''
 +
|-
 +
| ... || ...
 +
|-
 +
|}
 +
 +
This is discussed further at [[#Rigid Procedure]].
 +
 +
=== Singleton Pattern ===
 +
 +
The states in the second state pattern (above) are all singletons.  Fairly trivial.
 +
 +
=== Marginal ===
 +
 +
These, strictly speaking, are probably not quite design patterns, but they are very close, and follow similar ideas.
 +
 +
==== Builder Pattern ====
 +
 +
A [[Builder pattern]]-like idea is used to ''addName()''s to participants, whether they are collaborative or individual.  See [[#Logging Participants]].
 +
 +
==== Strategy Pattern ====
 +
 +
You might think, looking at state pattern number 1 (above) that it is a strategy.  This idea is discussed at [[#Logging Participants]].
 +
 +
=== Alternatives ===
 +
 +
Various other design patterns were considered during design.  A few of these, and why they were not included, are discussed below.
 +
 +
==== Composite Pattern ====
 +
 +
To model a pair of participants, or individual participants, I initially thought to use [[Composite pattern]].  However, this is no good - containment is too general (we have an exact number of participants each time).
 +
 +
==== Factory Method Pattern ====
 +
 +
[[Factory method pattern]] was considered to cope with the different panels when there are pairs or individuals.  This refactoring could still be done in the future, but at this stage it is not necessary, and instead I've decided to [[Keep it simple, stupid]].
 +
 +
==== Observer Pattern / Mediator Pattern ====
 +
 +
I considered using [[Observer pattern]] and [[Mediator pattern]] at one time, also.  See [[#Slide Show]].
 +
 +
== Design Maxims ==
 +
 +
In this section, I will outline some of the key maxims I have applied (or not applied, with reasons) in my project, and then discuss a couple of the biggest conflicts.  To see the actual design, so you can get an idea of these maxims in action, have a look at [[#Big Picture]].  The [[#Tour Of The Design]] also discusses these maxims in reference to specific cases where they are applied (I would recommend reading that first).  But they are (or at least should be) here too, for completeness.
 +
 +
=== Followed ===
 +
 +
==== Open Closed Principle ====
 +
 +
One of the goals of my design was for it to be extensible for similar experiments.  To do this, I aimed to follow the [[Open closed principle]].  However, it is not necessary for the entire design to follow this - I only picked the places where I most wanted extensibility - namely:
 +
# what ''Cards'' are displayed
 +
# what ''StartPanel'' is used
 +
# what types of ''Participant''(''Role'')s are possible
 +
# Ideally the ''TaskCard'' will also (but isn't yet, I'll see if I get time in the next...18 hours or so :)
 +
 +
Also, they may not be completely closed for modification (as [[Bob Martin]] says, we can never be 100% closed), but they're close enough for it to be worthwhile, but not so much that the design was over-complicated.  So how do these follow the [[Open closed principle]]?  Lets look at ''StartPanel'' and its subclasses.  By using an abstraction (''StartPanel'') we are open to extension - we just need to write a new subclass to ''StartPanel'' that implements the abstract methods to extend the functionality.  We are also partly closed to modification - adding a new ''StartPanel'' requires the subclass to follow the ''StartPanel'' abstraction that is fixed.  This means that creating a new subclass will not require any changes to other classes in the hierarchy, they only depend on the abstraction which, again, will not change.  So long as the appropriate methods are implemented, the card is free to do as it pleases, and possible behaviour is extended, without modifying existing functionality.  The other examples are similar.  In each case, an abstraction allows us to be open to extension, and closed to modification.
 +
 +
As discussed in PodCasts (#3), this principle conflicts with the concept to keep a design simple, and not attempt to predict the future.
 +
 +
==== Favour (yeah thats right, FavoUr) Composition Over Inheritance / Inheriting from GUI ====
 +
 +
My design initially used inheritance to create GUI elements.  Multiple classes extended ''JPanel'' to be able to use existing GUI behaviour.  Prompted by the [[Inheriting grom GUI]] discussion, I realised that inheritance was not necessary for my design.  I'm still sitting on the fence as to whether it is really any better either way, but I decided that, if all else fails, it's a safer bet to [[Favor composition over inheritance]].  Whether [[Liskov substitution principle]] was actually violated is debatable.  I would state that it wasn't - my ''EightPuzzlePanel'' (say) could be used in place of any ''JPanel'' and it would work.  The contract would not be violated - in no case was any behaviour restricted to more specific cases (ask for no more) or behaviour changed (deliver no less) (see [[Design by contract]]).  Really, I was just inheriting from a ''JComponent'' because I was lazy, it is habit, and it seems quicker (even though I'm sure it's no difference either way).  No behaviour needed to be extended, so I chose to [[Favor composition over inheritance]].  Avoiding inheritance has advantages: it makes reuse easier, it allows us to extend another class (e.g. Observable) if needed, and we're not depending on details of a superclass.
 +
 +
However, ''PuzzleButton'' is a different story, some inheritance remains.  Here, I want to subclass (rather than compose) because I want a ''PuzzleButton'' to inherit the ''addActionListener()'' behaviour.  Then, when an event is fired, the source is the ''PuzzleButton'', rather than the ''JButton'', and we can use the information wrapped up in the ''PuzzleButton'' like its position.  As explained above, I do not see this violating ''Liskov substitution principle]], the ''PuzzleButton'' can be used in place of ''JButton'' without violating the contract.  It may not be ideal, but as Blair Neate says "Its GUI, so anything goes".
 +
 +
==== Base classes are abstract ====
 +
 +
There is a whole class of maxims that state, more or less, that there should be [[No concrete base classes]] and [[The top of the class hierarchy should be abstract]] and [[Make base classes abstract]] ([[Riel's heuristics]]).  I have followed this in my design.  I have a few class hierarchies: the ''Card'' hierarchy (big), the ''ExperimentState'' hierarchy (fat) and the ''ParticipantRole'' hierarchy (barely noticable).  For each, the base class is abstract - but I've gone further than this.  Only my leaves in the inheritance hierarchy are concrete - nothing inherits from a concrete class.  What are the benefits from this approach?  We can't ever have instances of a class and a derived class where the subclass has '''changed''' the behaviour of its parent.  In fact, we can never have instances of a class and a subclass at all - the closest we can get is a concrete class that has a "sibling" in the class hierarchy sense.
 +
 +
==== Program to the Interface, not the Implementation ====
 +
 +
Here's a nice easy one - I make sure to [[Program to the interface not the implementation]].  This maxim applies in a number of situations.  Some examples:
 +
* ''ExperimentGUI'' uses a ''Map'' variable and is not concerned what specific implementation is used
 +
* ''ExperimentGUI'' stores a bunch of `Card`s.  It doesn't not what specific `Card` it has, only that it can call methods defined by its "interface".
 +
* A ''Participant'' doesn't know exactly what type of ''ParticipantRole''s it is using, it just calls ''addName()'' and ''printData()'' as needed.
 +
 +
==== Tell, Don't Ask + Law Of Demeter ===
 +
 +
I used [[Tell, don't ask]] and [[Law of demeter]] for most of my design (even though I am highly suspicious of the latter).  This is evidenced by the fact there are a number of "wrapper" method calls, or [[Recursion introduction]], for example the ''addPuzzlePanel()'' method can be called in ''EightPuzzlePanel'', which calls the method of the same name in the ''Participant'' class, which calls a method of the same name in a subclass of ''ParticipantRole''.  This is appropriate due to the fact there are not any large composition hierarchies, so the number of method wrappers are kept to a minimum.
 +
 +
==== Encapsulation Boundary, and Beware Accessors ====
 +
 +
I have used an object [[Encapsulation boundary]].  Have a look at ''Card'', ''StartPanel'', ''StartTestingPanel'' in [[#Big Picture]] for examples of where protected variables are used, so the subclasses can access them directly.  Unfortunately, Java's class encapsulation boundary means these attributes could be accessed by anything in the same package (ie everything).  Although this has potential for horrific things, I'm risking it based on the fact I'll make sure only competent, experienced programmers work on this application :).
 +
 +
Due to this object encapsulation boundary, I use [[Getters and setters]] sparingly.
 +
I think (especially for a relatively small design with not many developers) that it's good to [[Beware accessors]], at the risk of not following [[Minimize accesses to variables]].  Using [[Tell, don't ask]] and [[Law of Demeter]] (above) also results in less getters and setters.
 +
 +
==== Encapsulate That Which Varies ====
 +
 +
[[Encapsulate that which varies]] is occasionally used in my design, but more often combined with [[Design patterns]].  An example is how the participants can vary, either working individually or collaboratively.  The [[State pattern[[ allows us to encapsulate this variation.
 +
 +
==== CommentsSmell ====
 +
 +
I don't use many comments in my code.  This was partly because I wanted the code to be self-documenting (small methods with clear names, indicating what they do), partly because there are no complex algorithms or procedures, and partly because I was lazy.
 +
 +
==== Consciously deciding to use a language that does not support goto ====
 +
 +
And, if I'm as cool as Blair Neate, I may manage to follow [[Goto considered harmful]] ... Might be tricky ;)
 +
 +
 +
Unfortunately I have had some difficult following this, as I have a method that contains the word goto - ''gotoCard()''.  I fear this may be a costly mistake, but the alternative (a RenameMethod refactoring) seems too ambitious at this late stage.
 +
 +
=== Violated ===
 +
 +
==== Avoid Owner References and Composition Hierarchy ====
 +
 +
One design maxim I tried to follow, but couldn't quite, was [[Avoid owner references]].  There is a bidirectional association between ''Card''s and the ''ExperimentGUI''.  However, this two-way dependency is not too costly.  The relationship between them is well defined.  The ''ExperimentGUI'' keeps track of a bunch of ''Card''s that can be displayed.  By calling ''gotoCard()'', it can display whichever ''Card'' is currently appropriate.  However, the ''Card''s also need to know about the ''ExperimentGUI'' to notify it that they are done, and the next ''Card'' can now be displayed.  Could the bidirectional association have been made unidirectional?  Yes, but the cure would be worse than the symptoms.  One possibility is that I could make each "card" observable, to decrease the coupling between ''ExperimentGUI'' and a card. This doesn't fit the [[Observer pattern]] that well: there is only one object doing the observing, and many being observed. However, it would get rid of an owner reference. Is it really worth it when there's only one Observer, ''ExperimentGUI''? I don't think so. [[Observer pattern]] makes things a bit more complicated, and it isn't highly beneficial.  Broadcast updates aren't needed - there is only one dependent.
 +
 +
Besides, the ''ExperimentGUI'' is kind of like a controller, or a mediator in that it is a centralised class that (almost) everything communicates with, and it, primarily, just co-ordinates between the classes.  The two-way dependencies allow this centralised control to be possible.  The ''ExperimentGUI'' only uses the abstract class ''Card'' ([[Program to the interface not the implementation]]), so all it can do is call ''getPanel()'' or ''show()''.  We could improve this further by doing an [[Extract class]] refactoring on ''ExperimentGUI'', so ''Card''s can only call methods like ''panelDone()''.  So long as the interaction is well defined, this owner reference is not so costly.
 +
 +
I also had an intention to follow [[Composition hierarchy]], and did, other than the one dependency just described.
 +
 +
==== BewareSingletons ====
 +
 +
Although [[Arthur Riel]] advises us to [[Beware singletons]], I have some pretty harmless ones in my design.  Besides, if the [[Gang of four]] suggest them, they can't be all bad.  My only singletons are those from the [[State pattern]].  Yep, pretty harmless.
 +
 +
=== Conflicts ===
 +
 +
==== Do the simplest thing that could possibly work / Keep It Simple ====
 +
 +
Although I probably break this maxim as much as I follow it, I still follow it sometimes :).  In particular, my ''ActionListener''s in the ''StartPanel''s are just inner classes, as I found this to be the simplest solution.  In future, if there is more code duplication, they probably should be refactored.
 +
 +
[[Keep it simple]] would have to be the most violated of all principles in my design.  The worst aspect of my design is how complicated it is for something relatively simple.  This violates not only [[Keep it simple]], but [[Do the simplest thing that could possibly work]] and [[You ain't gonna need it]].  However, the complexity of my design is justified.  Lets face it, the simplest thing that could possibly work would be a single class that does everything.  ''SuperExperimentGUI''.  However, I think the simplest thing might not score very well in this assignment.  Additionally, it may work, but a nice design has other advantages.  It would be reusable, easier to understand, easier to optimise, it would have less code duplication and thus would require less maintenance.  More importantly, one of my main goals for this assignment was to have an extensible framework.  To be open to extension a bit of extra complexity is required.  There are abstractions that could be deemed unnecessary (e.g. ''Card'', ''StartPanel'', ''StartTestingPanel'', ...), and instead of the large StatePattern that adds an extra six classes, we could just have a big ugly if-else (-if-else-if-else-etc.).

Latest revision as of 04:20, 23 September 2008

Contents

Intro

My project is on my Cosc411 experiment. This experiment was exploring whether people can solve a problem-solving task quicker when collaborating. Participants would solve #The Eight Puzzle five times, either individually or in pairs (training phase), then solve it five times alone (testing phase). The experiment was mostly automated (so they'd just read instructions, click next, etc.). Their performance was logged.

This introduced a few design challenges:

  • Single partipants and pairs of participants can solve the puzzle in the training phase, so pairs and individuals need to be treated the same, but (for logging) produce different output
  • The whole experiment follows a rigid process, so we need a nice way to define what we show and when.
  • Moreover, participants can take one of three paths. Individual, Collaborative, or Collaborative-resume (when the pair splits up after training, one person can continue, the other jumps to the same stage).
  • Individuals or Pairs need to get slightly different instructions, but they are very similar.
  • The experiment is largely GUI based, but we want to separate the GUI out as much as possible.

Furthermore, I would like to be able to provide a framework from which one can easily extend the experiment if needed in the future. For example:

  • can add participants who train in groups of three
  • can solve a different puzzle
  • can add an extra slide of instructions

All the code was written this semester (while taking Cosc427) and I tried to make a good design following the ideas we were taught. So effectively, I started with nothing. However, after the experiment was run I decided to move into a refactoring stage (specifically for this 427 project) with two goals:

  1. to further improve the design (duh)
  2. to refactor the design such that (in a hypothetical situation) it could be easily extended for future similar collaboration experiments

You can see the design as it was before refactoring at #Old Design.

Contents

Code

The code is written in Java 5.0 (stupid version numbers), and you can get it here: media:427projectCode.tgz.

To run it, run `java ExperimentGui`. You can run collaborative mode with `-c` or collaborative resume with `-cr` flags.

The Eight Puzzle

The eight-puzzle is a simple game where there are 8 tiles and one blank arranged in a 3x3 grid. A tile can be moved if it is adjacent to the blank, as if it is being slid across (left, right, up or down). There is a similar fifteen-puzzle where the pieces are in a 4x4 grid.

In fact, a version of the puzzle appeared on the original Macintosh System, thanks to Andy Hertzfeld, and remained on the OS for a long time following. Anyways, I digress. You can see a screenshot of the puzzle below.

8 Puzzle

See wikipedia for more details...

Old Design

This is what my design looked like when the program was actually used to run the experiment. After this, I put on a refactoring hat for the purposes of this Cosc427 assignment. I've shown it here so I can talk about some of the changes made in design.

Firstly, let me apologise that it is so big, and that you have to scroll sideways :).

And here it is:

OldDesign.gif

Tour of the Design

Here, you get a step-by-step tour through the design. This is the logical journey that digests the design in small bites. Just click the links below, in order, for a look at the main parts of the design. It'll be built up bit-by-bit, and then looking at the ../BigPicture should make more sense. Note this is not necessarily a chronological order of how it was designed, only the final design is shown, built up piecewise, in the sequence that (I think) is easiest to comprehend.

  1. #Logging Participants
  2. #Slide Show
  3. #Rigid Procedure
  4. #Varied Instructions

Logging Participants

The experiment tested the performance of participants who trained collaboratively, or individually. After training, all participants worked individually for testing. This introduced the first design challenge. Single partipants or pairs of participants can solve the puzzle in the training phase, so pairs and individuals need to be treated the same, but (for logging) produce different output. Furthermore, a pair becomes (*alarm bells*) a single participant once training is complete. This is my proposed design to model participants:

Participants.gif

To clarify the UML, Phase (seen in the diagram in the Participant class) is an enum (see Enum Idiom) that takes one of two values: TRAINING or TESTING. The variable currPhase stores the current phase (ie if the participant is training or testing). The bottom region of a class shows the "properties" (anything for which there is a getWhatever() method). The two properties in Participant (currRole and time) signify there are corresponding getters, but neither of them are actually stored as variable: they are derived attributes.

As you can see, the Participant class models one or more people taking part in the experiment, and they have two Roles: a trainingRole and a testingRole (the singular class name Participant may be misleading - it can model more than one person if they are working together - however, making it plural is more confusing). The ParticipantRole handles how the log data is output, as defined by the printData() method. A PairRole will print out data for both of the participants, while an IndividualRole will print it out for the single individual. Also, an individual has a single "name" (not meant literally - just some identifier), while pairs have a couple of names, one per person. The reason why an IndividualRole stores a wasCollaborative variable is because pairs may split into individuals, but we still need to know they were previously pairs, for the log output.

Observant readers will recognise this is strikingly similar to the Strategy pattern - or is it State pattern? Hmmm. I originally was of the opinion it was Strategy Pattern. Each different implementation of the printData() algorithm is encapsulated in its own class, and thus is interchangeable. This means we can vary the number of people working on the experiment together, and still output data for each person. However this varies from the typical Strategy. The roles don't just encapsulate an algorithm, they also store additional data about the "role" - ie the name(s) and wasCollaborative for individuals (see below), and there's the addName() method (more on that soon). However, considering the fact that some participants change from being pairs to singles, perhaps it is a State pattern. When changing state from a single to a pair, the behaviour changes, like when changing class. Although the trainingRole or testingRole attributes never change class, this is just due to the currPhase variable - conceptually we are still changing state. Thanks to Phil, Blair and Greg for pointing this out at our late-night (err early-morning) discussion session :).

The main advantages of this approach (regardless of whether it is State or Strategy) are that we treat single participants and pairs uniformally, although they have slightly different behaviour, we Avoid becomes, and its easy to extend this design if we needed, say, a TripleRole for groups of three. Additionally, as an added bonus, the data about each participants two roles are stored like a history. For more design pattern discussion, see #Design patterns.


Tell, don't ask and Law of Demeter are followed - the Participant wraps up calls to the role. To do this Recursion introduction is used: the participant just relays calls to printData() and addName() to methods in one of its roles (of the same name), depending on the currPhase. In this situation I think applying these maxims is definitely beneficial. The ParticipantRole becomes transparent so we don't even need to know that a participant has a role, as far as anyone using a Participant is concerned, it can addName() and printData(), and we don't care how. It is effectively InformationHiding. The alternative, something like participant.getCurrentRole().addName(), requires the client to know that the participant has a role, it has a getter to access it, it stores the names, and we can change them using the method addName()...

So what is this addName() method? I needed some way to set a participants name. However, to set a pair's names, we would need two names, as opposed to the one required to set an individual's name. But if IndividualRole and PairRole have different setName() methods (with a different number of arguments), then a Participant could only setName() if it knew whether its role is training or testing. A Participant shouldn't need to know the types of its roles, just that it has roles, and what their interface is. Simply put, I want to Program to the interface not the implementation. To do this, a uniform way to set name(s) is required, even though we don't know how many names. I choose to do this by using a common interface: an addName() method sets one name, and it can just be called a couple of times to set a couple of names (for pairs). I noticed later that this was a similar idea to the Builder pattern (I don't think I can go so far as to say this is an application of the pattern) - we add parts (names) until we have built a product, and then we normally get the product (in my case, we change the product by adding parts, so there is no need to get it - we already had it). An alternative approach would have been to pass in a Collection of names. However, I decided this was not as good, as the number of items in the collection would be very small (and a majority of the time participants are individual), so there would be a lot of unnecessary Collections, and they do seem like overkill.

Slide Show

The experiment consists of pages of instruction, and the task to perform. Displaying these instructions, and the task itself, is analagous to a slide show. We have a number of slides (or "cards", thinking like Java's CardLayout) and for each, we show it, then move on. A key abstraction that was identified in the refactoring process, was that all panels that are displayed to the user abide by the same interface. Namely, all my panels follow a simple process - they show a card, until they are done. I decided a Card abstract class was necessary that captured this common behaviour.

The entire experiment application is controlled by the ExperimentGUI. This is also the main JFrame that contains any JPanel that is displayed. It keeps track of all the instruction Cards that can be displayed, plus the EightPuzzlePanel card that contains the gui by which someone solves the puzzle.

Card.gif

As you can see above, by providing a Card abstraction, the coupling is less tight - the ExperimentGUI depends on the abstraction rather than the specific panels. The abstract Card also gives us a uniform way to tell the ExperimentGUI to move to the next slide. The Card stores the ExperimentGUI and uses it in the (concrete) done() method. This method just calls the panelDone() method in ExperimentGUI.

What the instruction Cards are themselves is not very important. ExplanationPanel is just a generic panel, with a text box and a next button, for displaying any textual instructions to the user. The abstract StartPanel and StartTestingPanel classes are also shown. These, much as the name suggests, are panels for instructions when the experiment begins, and just before testing, respectively. We'll look at StartPanel a bit more later on.

To create a new Card, all that is needed is to provide implementations for the abstract method show() (for any behaviour when the Card is first displayed) and call the done() method when finished.


Dependency Inversion Principle, applied

How I ended up with this design deserves some explanation. You can see what I had prior to refactoring at #Old Design. I was concerned with the three two-way dependencies - each panel was tightly coupled to the ExperimentGUI. Firstly I considered using the Observer pattern. I said at the time:

'...I could make each "card" observable, to decrease the coupling between ExperimentGUI and a card. This doesn't fit the ObserverPattern that well: there is only one object doing the observing, and many being observed. However, it would get rid of an owner reference. Is it really worth it when there's only one Observer, ExperimentGUI? I think so. Observer pattern may make things a bit more complicated, but that's better than having the unnecesary owner reference: a card doesn't need to know its owner, only how to say "I'm done".'

Then I realised, there had to be a better way. Strike one.

My next thoughts were that I could use the Mediator pattern. I thought the interactions between the GUI and the Panels is well-defined (e.g. GUI says "show this panel", a panel says "I'm done"). But then, is the State really a state of the gui? Or a state of the Mediator? Upon further examination, I realised that the Mediator pattern also didn't really fit. Strike 2. The interactions between objects are well-defined, but not very complex. Applying mediator in its entirity would be overkill to the extreme, but looking at how the Mediator pattern was a good idea helped me to see the solution.

Finally, I realised, the ExperimentGUI was depending on the low-level details of each Card. What I needed, was to listen to Bob Martin and follow the Dependency inversion principle. He states that what is needed is a dependency inversion, so in my case, the ExperimentGUI would no longer depend on the panels, but on the Card abstraction, and the panels would also depend on this Card abstraction. Thus, the dependency is inverted. This gives us our design above!

ExperimentGUI still depends on EightPuzzlePanel :-/

You can see, however, that there is still one dependency to a subclass of the Card hierarchy - the ExperimentGUI must have an EightPuzzlePanel. This is due to the fact the EightPuzzlePanel is in charge of looking after the Participant. You could move the Participant to the ExperimentGUI controller class - but I think it makes more logical sense where it is. The EightPuzzlePanel is the only thing that needs to know about the participant - as they do the experiment (by using the panel) data about the participant is recorded. Here again we have an object (ExperimentGUI) depending on the low-level details (EightPuzzlePanel), which doesn't fit with the Dependency inversion principle. This would not be too difficult to fix. We need to represent the fact that we require some TaskCard (if you like) that is a card that represents the task to be completed by the Participant(s). A TaskCard would require a startTesting() method and a addName( method. I considered doing this, but didn't for two reasons. Firstly, it really isn't necessary yet. Do the simplest thing that could possibly work, Keep it simple and You ain't gonna need it were begging me not to. It would make more sense only to do it once a different puzzle was actually required to be plugged in, instead of the eight-puzzle. Secondly, I just ran out of time :) But hopefully you can see it would be easy to refactor, when necessary, and the goal of making the experiment extensible would still be possible.

Rigid Procedure

The process of participating in my experiment followed a rigid process. First you were presented with some starting instructions. Then you would go into a training phase. In the training phase you would, n times repeatedly, be presented with a "get ready screen" then the eight-puzzle itself. And the process continues. You can see that there are various stages of the experiment, and you step through them, so they could be represented as states. I decided to use the State pattern for modelling this.

State.gif

The merit of using the State pattern here is dubious. I have often weighed up whether, in this case, it is beneficial, and I've decided that marginally, yes, with some costs. The first thing to consider, is what behaviour is changing as you go through the states. Basically, the only thing is what Card is shown, and what state you go to next.

Using a state pattern definitely does get around a big ugly method containing many if-elses, and we should Beware value switches. Also, we can store additional information about each state (e.g. iteration) in the state class itself, following Keep behaviour with data. And, as the Gang of four say, it localises state-specific behaviour and partitions behaviour for different states. However, it also results in distributing the logic, relating to the one function, across many classes. As a result, the design is a lot less compact (as Martin Grenfell said, it's fat). Adding new states may make maintenance difficult. Also, ExperimentGUI coordinates all the functionality (the states just call methods on it, using an owner reference), so ExperimentGUI becomes bigger.

Weighing up all these pros and cons, I decided to use it.

Varied Instructions

When users start the experiment, they are presented with instructions. However, the instructions vary slightly for the individual or collaborative groups. For example, both groups get a welcome message, but the collaborative group needs a specific message saying how they will be working collaboratively. To achieve this, I used the Template method pattern.

Instructions.gif

The idea behind using this pattern was to create a general structure for the instructions, but allowing them to vary. Again this allows us to follow the Open closed principle. We are open to extend StartPanel, but we can only extend specific aspects of the instructions (namely the "enter name" message and the "process" message). We are closed to modifying any other instructions, the order of the instructions, or what the "welcome" and "process" messages are.

Again the question arises, is it really worth it? We could have an arguably simpler design where instead we have, say, a setInstructions() method and some constant Strings to make use of, e.g.

   public static const String WELCOME_STRING="Welcome to my experiment\n";


Instructions could then be set by (something like):

   setInstructions(WELCOME_STRING + "bla bla bla\n" + PROCESS_MESSAGE_STRING);

Using a Template method pattern is a lot more rigid - the order of instructions is clearly defined, and the common behaviour is factored out, so all StartPanels (ever) must have the fundamental welcome and process messages. In fact, the welcome and process messages are made private to ensure to subclasses can't override them.

The Big Picture

Here you can view the entire design, in all its glory. Note that the title was meant literally - the design is a very big picture size-wise. #Tour Of The Design gives a walk-through of the design if it doesn't seem to make sense. This diaagram is just to get an idea of where everything fits in. Also, this design is often referred to, so feel free to print it off :).

File:BigPicture.jpg

Design Patterns

The following design patterns are present in my design.

Template Method Pattern

A Template method pattern is used to vary instructions depending on the participant's collaboration condition, but still following a uniform structure. It is discussed more at #Varied Instructions.

Participants:

AbstractClass StartPanel
concrete templateMethod() getInstructions()
abstract primitiveOperation1() getEnterNameMessage()
abstract primitiveOperation2() getConditionMessage()
ConcreteClass1 IndividualStartPanel
ConcreteClass2 CollaborativeStartPanel
ConcreteClass3 CollaborativeTestingStartPanel

State Pattern Number 1

A Participant has a ParticipantRole that follows the State pattern. This allows participants to be individuals or pairs, behave differently depending on which, and change from one to the other. This could easily be considered a Strategy pattern however - this and related discussion is at #Logging Participants.

Participants:

Context Participant
State ParticipantRole
abstract handle() printData()
ConcreteState1 subclass IndividualRole
ConcreteState2 subclass PairRole

addName() could be considered a second handle() method, but I won't describe it here. I think it is more like a Builder pattern below...

State Pattern Number 2

A State pattern is also used for the various stages of the experiment.

Participants:

Context ExperimentGUI
State ExperimentState
abstract handle() showNextCard()
ConcreteState1 subclass TrainingInstructionsState
ConcreteState2 subclass GetReadyState
ConcreteState3 subclass FinishState
... ...

This is discussed further at #Rigid Procedure.

Singleton Pattern

The states in the second state pattern (above) are all singletons. Fairly trivial.

Marginal

These, strictly speaking, are probably not quite design patterns, but they are very close, and follow similar ideas.

Builder Pattern

A Builder pattern-like idea is used to addName()s to participants, whether they are collaborative or individual. See #Logging Participants.

Strategy Pattern

You might think, looking at state pattern number 1 (above) that it is a strategy. This idea is discussed at #Logging Participants.

Alternatives

Various other design patterns were considered during design. A few of these, and why they were not included, are discussed below.

Composite Pattern

To model a pair of participants, or individual participants, I initially thought to use Composite pattern. However, this is no good - containment is too general (we have an exact number of participants each time).

Factory Method Pattern

Factory method pattern was considered to cope with the different panels when there are pairs or individuals. This refactoring could still be done in the future, but at this stage it is not necessary, and instead I've decided to Keep it simple, stupid.

Observer Pattern / Mediator Pattern

I considered using Observer pattern and Mediator pattern at one time, also. See #Slide Show.

Design Maxims

In this section, I will outline some of the key maxims I have applied (or not applied, with reasons) in my project, and then discuss a couple of the biggest conflicts. To see the actual design, so you can get an idea of these maxims in action, have a look at #Big Picture. The #Tour Of The Design also discusses these maxims in reference to specific cases where they are applied (I would recommend reading that first). But they are (or at least should be) here too, for completeness.

Followed

Open Closed Principle

One of the goals of my design was for it to be extensible for similar experiments. To do this, I aimed to follow the Open closed principle. However, it is not necessary for the entire design to follow this - I only picked the places where I most wanted extensibility - namely:

  1. what Cards are displayed
  2. what StartPanel is used
  3. what types of Participant(Role)s are possible
  4. Ideally the TaskCard will also (but isn't yet, I'll see if I get time in the next...18 hours or so :)

Also, they may not be completely closed for modification (as Bob Martin says, we can never be 100% closed), but they're close enough for it to be worthwhile, but not so much that the design was over-complicated. So how do these follow the Open closed principle? Lets look at StartPanel and its subclasses. By using an abstraction (StartPanel) we are open to extension - we just need to write a new subclass to StartPanel that implements the abstract methods to extend the functionality. We are also partly closed to modification - adding a new StartPanel requires the subclass to follow the StartPanel abstraction that is fixed. This means that creating a new subclass will not require any changes to other classes in the hierarchy, they only depend on the abstraction which, again, will not change. So long as the appropriate methods are implemented, the card is free to do as it pleases, and possible behaviour is extended, without modifying existing functionality. The other examples are similar. In each case, an abstraction allows us to be open to extension, and closed to modification.

As discussed in PodCasts (#3), this principle conflicts with the concept to keep a design simple, and not attempt to predict the future.

Favour (yeah thats right, FavoUr) Composition Over Inheritance / Inheriting from GUI

My design initially used inheritance to create GUI elements. Multiple classes extended JPanel to be able to use existing GUI behaviour. Prompted by the Inheriting grom GUI discussion, I realised that inheritance was not necessary for my design. I'm still sitting on the fence as to whether it is really any better either way, but I decided that, if all else fails, it's a safer bet to Favor composition over inheritance. Whether Liskov substitution principle was actually violated is debatable. I would state that it wasn't - my EightPuzzlePanel (say) could be used in place of any JPanel and it would work. The contract would not be violated - in no case was any behaviour restricted to more specific cases (ask for no more) or behaviour changed (deliver no less) (see Design by contract). Really, I was just inheriting from a JComponent because I was lazy, it is habit, and it seems quicker (even though I'm sure it's no difference either way). No behaviour needed to be extended, so I chose to Favor composition over inheritance. Avoiding inheritance has advantages: it makes reuse easier, it allows us to extend another class (e.g. Observable) if needed, and we're not depending on details of a superclass.

However, PuzzleButton is a different story, some inheritance remains. Here, I want to subclass (rather than compose) because I want a PuzzleButton to inherit the addActionListener() behaviour. Then, when an event is fired, the source is the PuzzleButton, rather than the JButton, and we can use the information wrapped up in the PuzzleButton like its position. As explained above, I do not see this violating Liskov substitution principle]], the PuzzleButton can be used in place of JButton without violating the contract. It may not be ideal, but as Blair Neate says "Its GUI, so anything goes".

Base classes are abstract

There is a whole class of maxims that state, more or less, that there should be No concrete base classes and The top of the class hierarchy should be abstract and Make base classes abstract (Riel's heuristics). I have followed this in my design. I have a few class hierarchies: the Card hierarchy (big), the ExperimentState hierarchy (fat) and the ParticipantRole hierarchy (barely noticable). For each, the base class is abstract - but I've gone further than this. Only my leaves in the inheritance hierarchy are concrete - nothing inherits from a concrete class. What are the benefits from this approach? We can't ever have instances of a class and a derived class where the subclass has changed the behaviour of its parent. In fact, we can never have instances of a class and a subclass at all - the closest we can get is a concrete class that has a "sibling" in the class hierarchy sense.

Program to the Interface, not the Implementation

Here's a nice easy one - I make sure to Program to the interface not the implementation. This maxim applies in a number of situations. Some examples:

  • ExperimentGUI uses a Map variable and is not concerned what specific implementation is used
  • ExperimentGUI stores a bunch of `Card`s. It doesn't not what specific `Card` it has, only that it can call methods defined by its "interface".
  • A Participant doesn't know exactly what type of ParticipantRoles it is using, it just calls addName() and printData() as needed.

= Tell, Don't Ask + Law Of Demeter

I used Tell, don't ask and Law of demeter for most of my design (even though I am highly suspicious of the latter). This is evidenced by the fact there are a number of "wrapper" method calls, or Recursion introduction, for example the addPuzzlePanel() method can be called in EightPuzzlePanel, which calls the method of the same name in the Participant class, which calls a method of the same name in a subclass of ParticipantRole. This is appropriate due to the fact there are not any large composition hierarchies, so the number of method wrappers are kept to a minimum.

Encapsulation Boundary, and Beware Accessors

I have used an object Encapsulation boundary. Have a look at Card, StartPanel, StartTestingPanel in #Big Picture for examples of where protected variables are used, so the subclasses can access them directly. Unfortunately, Java's class encapsulation boundary means these attributes could be accessed by anything in the same package (ie everything). Although this has potential for horrific things, I'm risking it based on the fact I'll make sure only competent, experienced programmers work on this application :).

Due to this object encapsulation boundary, I use Getters and setters sparingly. I think (especially for a relatively small design with not many developers) that it's good to Beware accessors, at the risk of not following Minimize accesses to variables. Using Tell, don't ask and Law of Demeter (above) also results in less getters and setters.

Encapsulate That Which Varies

Encapsulate that which varies is occasionally used in my design, but more often combined with Design patterns. An example is how the participants can vary, either working individually or collaboratively. The [[State pattern[[ allows us to encapsulate this variation.

CommentsSmell

I don't use many comments in my code. This was partly because I wanted the code to be self-documenting (small methods with clear names, indicating what they do), partly because there are no complex algorithms or procedures, and partly because I was lazy.

Consciously deciding to use a language that does not support goto

And, if I'm as cool as Blair Neate, I may manage to follow Goto considered harmful ... Might be tricky ;)


Unfortunately I have had some difficult following this, as I have a method that contains the word goto - gotoCard(). I fear this may be a costly mistake, but the alternative (a RenameMethod refactoring) seems too ambitious at this late stage.

Violated

Avoid Owner References and Composition Hierarchy

One design maxim I tried to follow, but couldn't quite, was Avoid owner references. There is a bidirectional association between Cards and the ExperimentGUI. However, this two-way dependency is not too costly. The relationship between them is well defined. The ExperimentGUI keeps track of a bunch of Cards that can be displayed. By calling gotoCard(), it can display whichever Card is currently appropriate. However, the Cards also need to know about the ExperimentGUI to notify it that they are done, and the next Card can now be displayed. Could the bidirectional association have been made unidirectional? Yes, but the cure would be worse than the symptoms. One possibility is that I could make each "card" observable, to decrease the coupling between ExperimentGUI and a card. This doesn't fit the Observer pattern that well: there is only one object doing the observing, and many being observed. However, it would get rid of an owner reference. Is it really worth it when there's only one Observer, ExperimentGUI? I don't think so. Observer pattern makes things a bit more complicated, and it isn't highly beneficial. Broadcast updates aren't needed - there is only one dependent.

Besides, the ExperimentGUI is kind of like a controller, or a mediator in that it is a centralised class that (almost) everything communicates with, and it, primarily, just co-ordinates between the classes. The two-way dependencies allow this centralised control to be possible. The ExperimentGUI only uses the abstract class Card (Program to the interface not the implementation), so all it can do is call getPanel() or show(). We could improve this further by doing an Extract class refactoring on ExperimentGUI, so Cards can only call methods like panelDone(). So long as the interaction is well defined, this owner reference is not so costly.

I also had an intention to follow Composition hierarchy, and did, other than the one dependency just described.

BewareSingletons

Although Arthur Riel advises us to Beware singletons, I have some pretty harmless ones in my design. Besides, if the Gang of four suggest them, they can't be all bad. My only singletons are those from the State pattern. Yep, pretty harmless.

Conflicts

Do the simplest thing that could possibly work / Keep It Simple

Although I probably break this maxim as much as I follow it, I still follow it sometimes :). In particular, my ActionListeners in the StartPanels are just inner classes, as I found this to be the simplest solution. In future, if there is more code duplication, they probably should be refactored.

Keep it simple would have to be the most violated of all principles in my design. The worst aspect of my design is how complicated it is for something relatively simple. This violates not only Keep it simple, but Do the simplest thing that could possibly work and You ain't gonna need it. However, the complexity of my design is justified. Lets face it, the simplest thing that could possibly work would be a single class that does everything. SuperExperimentGUI. However, I think the simplest thing might not score very well in this assignment. Additionally, it may work, but a nice design has other advantages. It would be reusable, easier to understand, easier to optimise, it would have less code duplication and thus would require less maintenance. More importantly, one of my main goals for this assignment was to have an extensible framework. To be open to extension a bit of extra complexity is required. There are abstractions that could be deemed unnecessary (e.g. Card, StartPanel, StartTestingPanel, ...), and instead of the large StatePattern that adds an extra six classes, we could just have a big ugly if-else (-if-else-if-else-etc.).

Personal tools