|
|
| (13 intermediate revisions by 5 users not shown) |
| Line 12: |
Line 12: |
| | * ''Toad''s eat ''AdultFrogs''. | | * ''Toad''s eat ''AdultFrogs''. |
| | | | |
| − | == Broken heuristics == | + | == Criticisms == |
| | | | |
| − | * [[Avoid becomes]] - ''Frog'' phases of life are modeled by inheritance: ''Egg'' ''Tadpole'', ''AdultFrog''. | + | * [[2009 frog design criticisms]] |
| − | * [[Beware type switches]] - the ''phase'' variable in ''Frog'' is based on the type of an object (''Frog'').
| + | |
| − | * [[Beware value switches]] - the ''swim()'' method in ''Tadpole'' differs in behavior based on the value of an attribute (''type'').
| + | |
| − | * [[Avoid no-op overrides]] ("Design by Contract") - the derived class ''Egg'' overrides the base class's ''hop()'' and ''swim()'' methods with methods which does nothing.
| + | |
| − | * [[One key abstraction]] - the ''Move'' interface contains more than one key abstraction. It contains methods to move such as ''hop()'' and ''swim()'', but it also contains a ''display()'' method which is a separate key abstraction.
| + | |
| | | | |
| − | == "Feels Bad" ==
| + | * [[2010 frog design criticisms]] |
| − | | + | |
| − | * ''Toad'' is a subclass of ''AdultFrog''. | + | |
| − | * ''FrogBrain'' contains an array of ''Egg''.
| + | |
| − | | + | |
| − | :''Why do these things feel bad? --[[User:Warwick Irwin|Wal]]''
| + | |
| − | | + | |
| − | == Go export yourself ==
| + | |
| − | | + | |
| − | Should a ''Frog'' be able to export itself?
| + | |
| − | | + | |
| − | [[image:frog.jpg]]
| + | |
| − | | + | |
| − | Our discussion of this was quite polarized. Some thought the current "frog.exportXML()" arrangement was consistent with the [[Keep related data and behaviour together]] heuristic, seemed the most intuitive way of calling the method, and thus fine how it was. Others expressed concern that having many classes with an "exportXML" method was not elegant, and would result in unnecessary repetition. Also, such methods violate the principles of [[One key abstraction]], [[Separation of concerns]], and the [[Single Responsibility Principle]].
| + | |
| − | | + | |
| − | Criticism is of little value unless a better solution can be proposed. Hence the latter camp suggested the [[Visitor|Visitor pattern]]. Under their proposal, XML exporting methods would be grouped in an XMLVisitor class. To export XML, the class would be used in this form:
| + | |
| − | | + | |
| − | frog.accept(xmlVisitor)
| + | |
| − | | + | |
| − | This is certainly a viable solution, and has the advantage of grouping all XML export methods within a single convenient class.
| + | |
| − | | + | |
| − | At this point we noted an intrinsic conflict between the design maxims. Separation of concerns seems to strongly contradict the idea that we must keep related data and behaviour together. We must then ask what we mean by "related", and what exactly defines a "concern".
| + | |
| − | | + | |
| − | My view was that XML-exporting methods for various objects are related in a technical sense (they work with the same format) but not in an object-oriented sense (they export totally different objects). The raison d'etre of object-oriented design is to group parts of a program on the basis of their real-world and logical relationships, rather than their technical relationships. Consequently, we ought to group a method that exports frogs in XML with frog-related program components, rather than XML-related program components. Thus the first solution seems more sensible to me.
| + | |
| − | | + | |
| − | Duplication of XML-exporting code can then be avoided by using an XMLExporter class that frog.exportXML and other exportXML methods can use behind the scenes within the implementation of that method.
| + | |
| − | | + | |
| − | --[[User:RobertLechte|Robert]] 06:18, 31 July 2008 (UTC)
| + | |
| − | | + | |
| − | For an example of the Visitor pattern in action, see [[A froggy visitor]] --[[User:Tureiti Keith|TK]] 06:20, 2 August 2008 (UTC)
| + | |
| − | | + | |
| − | ==The Redesign: <tt>step one</tt>==
| + | |
| − | | + | |
| − | While it is far from ideal here is a step towards a better design (In my opinion).
| + | |
| − | | + | |
| − | What have I done?
| + | |
| − | | + | |
| − | I have addressed the violated heuristic [[Beware type switches]] and [[Avoid becomes]] by replacing the storage of the phase in the ''Frog'' class with a [[State]] [[design patterns|design pattern]] using the ''Frog'' as the context and ''Egg'', ''Tadpole'', ''AdultFrog'' as concrete states for the adstract state ''FrogLifePhase' [[image:FrogRedesign1.jpg|thumb|Click to enlarge]]
| + | |
| − | '
| + | |
| − | While it still encourages the use of [[Avoid no-op overrides|no-op's]] the ''FrogLifePhase'' now implements the interface ''Move''
| + | |
| − | | + | |
| − | A new Abstract class ''Amphibian'' has been introduced to better represent the relationship between ''Frog'' and ''Toad''.
| + | |
| − | | + | |
| − | A better place for the list of all frogs to be kept might be in the ''Biologist'' class so i thave moved it there from the ''Toad'' class.
| + | |
| − | | + | |
| − | Some things may just not make sense now like keeping a list of all the eggs when it is probable that there will only be one instance of the ''Egg'' class shaired by all ''Frog'' classes.
| + | |
| − | | + | |
| − | | + | |
| − | | + | |
| − | [[User:Jason Clutterbuck|Jason Clutterbuck]] 06:59, 31 July 2008 (UTC)
| + | |
