Cleanest choice for symmetric operations

Question

I have a need to do some processing from a format A to a format B and from B to A. The job in one direction is very similar to its counterpart. Both formats are represented with an interface Msg.

In such a case, I can see four obvious solutions, which is the cleanest? I hope there are some concrete principles explaining one choice over the others and not just personal preferences

Here are the obvious choices

1) Different classes for each

public class TransformToA {
   public TransformToA() {
       ...
   }
   public Msg transform(Msg incoming) {
       ...
   }
}
public class TransformToB {
   public TransformToB() {
       ...
   }
   public Msg transform(Msg incoming) {
       ...
   }
}

Note that in this option, I could extract some common logic into a third common class to avoid code duplication

2) A boolean field to define the direction

public class Transformer {
   private boolean toBFormat;
   public Transformer(boolean toBFormat) {
       ...
   }
   public Msg transform(Msg incoming) {
       if (toBFormat) {
          ...
       } else {
          //to A format
       }
   }
}

3) a boolean flag on the method (this is probably the worst since the caller is forced to pass the flag every single time and makes a method behave in two different ways)

public class Transformer {
   public Transformer() {
       ...
   }
   public Msg transform(Msg incoming, boolean toBFormat) {
       if (toBFormat) {
          ...
       } else {
          //to A format
       }
   }
}

4) Two different methods

public class Transformer {
   public Transformer() {
       ...
   }
   public Msg transformToA(Msg incoming) {
       ...
   }
   public Msg transformToB(Msg incoming) {
       ...
   }
}

You missed one: overloading the transform method, so that when you pass an `A` it will produce a `B` and vice versa. — Kilian Foth, Nov 05 '14 at 15:08
one may argue that conceptually, this has been addressed in [How would you know if you've written readable and easily maintainable code?](http://programmers.stackexchange.com/a/141010/31260) If your peers keep complaining about your way of doing things, be it one way or another, you better change to make them feel better — gnat, Nov 05 '14 at 15:18
Booleans make awful function arguments. Besides the fact that they inevitably mean the function does two things instead of one, it doesn't scale when you need to do 3+ things in the future, and it's stupidly opaque. In a function call like `doSomething(true)`, the boolean tells me nothing at face value. Anyways, why not establish a common intermediate format and then give each message type the ability to translate its contents to and from that format? It avoids an explosion of combinations and lets you look at private fields if needed. — Doval, Nov 05 '14 at 15:55
@Doval true that. To dot an i, it's usually better to use an enum with well named values; a possibility OP didn't mention. — Konrad Morawski, Dec 05 '14 at 23:31

score 2 · Answer 1 · answered Nov 05 '14 at 16:13

You're describing a sort of copy constructor: using one Msg instance to construct another. I would implement it as such, finding another place to put common helper functionality.

public class A implements Msg {
  public A(Msg msg){...}
}

public class B implements Msg {
  public B(Msg msg){...}
}

However, you've been vague about your reasons for doing the transformation, and I suspect there's another issue going on. It feels like you might be trying to do too much in the A and B classes. Perhaps you should split Msg into its own class that acts only as a model, then have A and B act more as a view, with only the functionality where it matters what format it's in.

For example, if Msg was a spreadsheet document, and A and B are Excel or OpenDocument formats, then you would want to do the vast majority of your work in your internal Msg format, and only create an A or B when you're reading or writing your document to disk.

score 0 · Answer 2 · answered Jan 05 '15 at 10:20

Many clean solution exist.

My pick in Java would definitely be the following structure:

public abstract class AbstractTransform {

    ... do whatever is the same in all transformer, 
        or if the constructor would be the same etc...

    protected void iDoThisAllTheTime(){
        ... boring, repeating codes come here
    }

    public Msg transform(Msg incoming);

}

then just extend this abtract class, and implement the missing method:

public class TransformToA extends AbstractTransform {

    @Override
    public Msg transform(Msg incoming){
        ... do the transformation
        iDoThisAllTheTime();
    }

}

Advatanges:

easy to extend in a way you want. If you need some additional one, just extend once more.
documents itself, you need to document pretty much only the Abstract one to say what is it for. The rest of classes will do the same.
if you need common logics, you can implement it in abstract class, you will avoid duplications. You can implement protected methods for common use and invoke them in child classes.
not complicated. You don't need to juggle with classes, interfaces if the task is simple, your code can stay simple.

Cleanest choice for symmetric operations

2 Answers2