Geek Answers Handbook

Hell interface or acceptable design?

I am refactoring some inherited code that repeated the same thing many times using case case:

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 it could become

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

However, I found that some elements do not even use all the properties. So, I started thinking about something more similar:

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

and you can create a class like this:

 public class MyClassUsesProp2And3 : InterfaceForProp2, InterfaceForProp3

However, I am afraid that I will over-fragment this code and it may scroll too much. Maybe I should not be too afraid of what will be essentially one method interface, but I wanted to see if I have a design template before I go this way? (the only ones that arose in my head, but were not entirely suitable, were Decorator or Composite )

UPDATE

All properties are unique types.

Ultimately, it is an injection form of addiction. The code is too confusing to use something like Ninject right now, but in the end I can even get rid of some of them and use an injection container. Currently, there is also some logic that runs outside of the variable setting. This is all legacy code, and I'm just trying to clean it up a bit.

+4
source share
4 answers

Basically you want to make the actors (the type with setProp methods) the same type of "Actor" and make the properties (prop1 ... n) the same type of "Prop". This will reduce your code to

 actor.setProp(prop)

If you want to avoid using instanceOf, the only way I can come up with is to go with the visitor pattern by making Prop the visitor. I would also use the template method to make my life easier. In Java, I would do it like this (for two real 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 things like this:

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

... it's super nice!

+1
source

I do not know if there is a “correct” answer to this question, but here is what I will do.

 class Properties { prop1 prop2 prop3 } interface PropertySetable { setProperties(Properties prop); } public class MyClassUsesProp2And3 implements PropertySetable { setProperties(Properties prop) { //I know I need only 2 and 3 myProp2 = prop.prop2; myProp3 = prop.prop3; } }

When calling a function, you should not have a role.

  someFunc(..., PropertySetable, Properties,...) { PropertySetable.setProperties(Properties); }

This is the basic structure.

You must encapsulate properties - make properties private and have appropriate constructors. Or use the Builder template to create properties ... and much more.

+1
source

I think the answer depends on why you want to reorganize the code in the first place.

  • If you want to completely get rid of the switch block, you will have to implement an interface with one method using three arguments. Each class would have to figure out which arguments apply and what to do with these arguments.
  • If you want to reduce the total amount of code, I’m not sure that adding the property logic of a class property is not smaller than the original switch statement
  • If you want to use the editor code completion feature to insert the correct method with the right amount of / args properties, then just implement the correct method for each class and generally avoid the interface.
+1
source

The visitor template is a standard solution when you come across switch and typecasts operators. Your code for each case goes into a separate method in the visitor class. And your classes will implement only one interface - the accept method for receiving visitors. You will have more code than you have now, but it will read much more cleanly.

+1
source

More articles:


All Articles