An obvious case of a CodeSmell is a bunch of almost similar subclasses of a class.
Say you have different classes for different inputs that act the same way, and you are having a lot of replicate code (similar methods) in each subclass to make the input look uniform. Restructure the work between the parent(s) and subclasses.
"Raise" the replicate parts to the parent class, and differentiate the cases represented by subclasses barely with a state attribute used for flow control on differing parts. Depending on amount of differences, you might want to either squeeze them into separate functions inside the parent(s) or keep the subclasses, but just for the special work. ListenToTheCode.
Another case requiring a state instead of a bunch of behaviors might be a bunch of behaviors of one class that have to be chosen between by the caller depending on the internal state of an object of that class. Make a single behavior that inspects your state attribute and passes the call to an appropriate behavior.
I'm sorry, but this solution smells worse to me than the original problem. I followed you right up to the part about "differentiate the cases represented by subclasses with a state attribute used for flow control". Why would you use an attribute? Why not just try to refactor your methods so that ONLY the parts that are different are delegated to the subclasses?
And looking at the second part, that sounds to me like you might have a case where the different behaviors should be part of the object that makes up the internal state. In that case you could use polymorphism rather than a switch statement.
I might be missing something here. Could you post some sample code that illustrates a case where this idiom is useful?
-- KyleBrown
I agree that you should differentiate the pieces of code to be executed within the same class by evaluating some piece of the state of that class. Why ? Simply because this is proposed by the StrategyPattern (GangOfFour). However, the code itself would be placed in other classes: one class per logical algorithm (these have a common ancestor to define the interface, which should be the same for all of these classes containing the actual code). Depending on the state of your class, one of the classes, containing the code to be executed in that case, would be instantiated. Read more on the StrategyPattern to get a clearer picture.
-- RobinLeysen
Trust me, I know the StrategyPattern. But I still don't get this, or the link between it and the StrategyPattern. In my implementations of the StrategyPattern (see the DesignPatternsSmalltalkCompanion) I never had to put state in the superclass to make the subclasses operate correctly. And I don't believe that the GangOfFour ever suggested that the superclass strategy instantiate the subclasses...that would have been the job of the "Context" object in the StrategyPattern. -- KyleBrown
Example: [In Java] Instead of:
public abstract class HtmlTag {
abstract public String toString();}
public class Form extends HtmlTag {
public Form() { super(); } public String toString() { return ( "<form " ); }}
public class Table extends HtmlTag {
public Table() { super(); } public String toString() { return ( "<table " ); }}
Better is:
public class HtmlTag {
protected String _tagName; public HtmlTag( String tagName ) { _tagName = tagName; } public String toString() { return ( "<" + _tagName + " " ); }}
public class Form extends HtmlTag {
public Form() { super( "form" ); }}
public class Table extends HtmlTag {
public Table() { super( "table" ); }}
Is how I interpret the explanation.
Gerrit
[CategoryRefactoring, and a word in the RefactoringLanguage]