Search code examples
design-patternsinterfacestrategy-pattern

Interface Hell or Acceptable Design?


I am refactoring some legacy code that was doing close to the same thing over and over via a case statement:

switch(identifier)
    case firstIdentifier: 
        (SomeCast).SetProperties(Prop1,Prop2,Prop3);
        break;
    ...
    case anotherIdentifier:
        (SomeDifferentCast).SetProperties(Prop1, Prop2, Prop3);
        break;

So, I tried to create a unique interface so that this could become

(SameInterfaceCast).SetProperties(Prop1,Prop2,Prop3);

However, I then found that some items do not even use all of the properties. So, I began to think of something more like this:

if(item is InterfaceForProp1)
    (InterfaceForProp1).SetProp1(Prop1);
if(item is InterfaceForProp2)
    (InterfaceForProp2).SetProp2(Prop2);
if(item is InterfaceForProp3)
    (InterfaceForProp3).SetProp3(Prop3);

and you could create a class like this:

public class MyClassUsesProp2And3 : InterfaceForProp2, InterfaceForProp3

However, I am afraid that I am overly fragmenting this code and it could balloon too much. Maybe I should not be too fearful of what will essentially be one method interfaces, but I wanted to see if I am missing a design pattern before going down this path? (the only ones that popped into my mind but were not quite the right fit were the Decorator or Composite patterns)

UPDATE

All properties are unique types.

Ultimately, this is a form of dependency injection. The code is too messed up to use something like Ninject right now, but eventually I might even be able to get rid of some of these and use an injection container. There is also currently some logic that is being done beyond just setting a variable. This is all legacy code, and I am just trying to clean it up little by little.


Solution

  • Basically you want to make the actors (type with setProp methods) the same type 'Actor' AND make the properties (prop1...n) the same type 'Prop'. This would reduce your code to

    actor.setProp(prop)
    

    If you want to avoid using instanceOf, the only way I can think of is to go with the Visitor pattern, making 'Prop' the visitor. I would also use template method to make my life easier. In Java I would make it look like this (for two actual kinds of Prop).

    class Actor {
    
        protected void set(Prop1 p1) {
            // Template method, do nothing
        }
    
        protected void set(Prop2 p2) {
            // Template method, do nothing
        }
    
        public void setProp(Prop p) {
            p.visit(this);
        }
    
        public interface Prop {
            void visit(Actor a);
        }
    
        public static Prop makeComposite(final Prop...props ) {
            return new Prop() {
    
                @Override
                public void visit(final Actor a) {
                    for (final Prop p : props) {
                        p.visit(a);
                    }
                }
            };
        }
    
        public static class Prop1 implements Prop {
            public void visit(Actor a) {
                a.set(this);
            }
        }
    
        public static class Prop2 implements Prop {
            public void visit(Actor a) {
                a.set(this);
            }
        }
    }
    

    This allows you to do stuff like this:

        ConcreteActor a = new ConcreteActor();
        Prop p = Actor.makeComposite(new ConcreteProp1(42), new ConcreteProp2(-5));
        a.setProp(p);
    

    ...which is super nice!