views:

572

answers:

4

This question is specifically regarding C#, but I am also interested in answers for C++ and Java (or even other languages if they've got something cool).

I am replacing switch statements with polymorphism in a "C using C# syntax" code I've inherited. I've been puzzling over the best way to create these objects. I have two fall-back methods I tend to use. I would like to know if there are other, viable alternatives that I should be considering or just a sanity check that I'm actually going about this in a reasonable way.

The techniques I normally use:

  1. Use an all-knowing method/class. This class will either populate a data structure (most likely a Map) or construct on-the-fly using a switch statement.
  2. Use a blind-and-dumb class that uses a config file and reflection to create a map of instances/delegates/factories/etc. Then use map in a manner similar to above.
  3. ???

Is there a #3, #4... etc that I should strongly consider?


Some details... please note, the original design is not mine and my time is limited as far as rewriting/refactoring the entire thing.

Previous pseudo-code:

public string[] HandleMessage(object input) {

   object parser = null;
   string command = null;
   if(input is XmlMessage) {
      parser = new XmlMessageParser();
      ((XmlMessageParser)parser).setInput(input);
      command = ((XmlMessageParser)parser).getCommand();
   } else if(input is NameValuePairMessage) {
      parser = new NameValuePairMessageParser();
      ((NameValuePairMessageParser)parser).setInput(input);
      command = ((XmlMessageParser)parser).getCommand();
   } else if(...) {
      //blah blah blah
   }

   string[] result = new string[3];
   switch(command) {
      case "Add":
          result = Utility.AddData(parser);
          break;
      case "Modify":
          result = Utility.ModifyData(parser);
          break;
      case ... //blah blah
          break;
   }
   return result;
}

What I plan to replace that with (after much refactoring of the other objects) is something like:

public ResultStruct HandleMessage(IParserInput input) {
   IParser parser = this.GetParser(input.Type);       //either Type or a property
   Map<string,string> parameters = parser.Parse(input);
   ICommand command = this.GetCommand(parameters);  //in future, may need multiple params 
   return command.Execute(parameters);              //to figure out which object to return.
}

The question is what should the implementation of GetParser and GetCommand be?

Putting a switch statement there (or an invokation of a factory that consists of switch statements) doesn't seem like it really fixes the problem. I'm just moving the switch somewhere else... which maybe is fine as its no longer in the middle of my primary logic.

A: 

I don't see any problems with a message handler like you have it. I certainly wouldn't go with the config file approach - why create a config file outside the debugger when you can have everything available at compile time?

Jess
+1  A: 

You can have an

public interface IParser<SomeType> : IParser{}

And set up structuremap to look up for a Parser for "SomeType"

It seems that Commands are related to the parser in the existing code, if you find it clean for your scenario, you might want to leave that as is, and just ask the Parser for the Command.

Update 1: I re-read the original code. I think for your scenario it will probably be the least change to define an IParser as above, which has the appropiate GetCommand and SetInput.

The command/input piece, would look something along the lines:

    public string[] HandleMessage<MessageType>(MessageType input) {
       var parser = StructureMap.GetInstance<IParser<MessageType>>();
       parser.SetInput(input);
       var command = parser.GetCommand();
       //do something about the rest
   }

Ps. actually, your implementation makes me feel that the old code, even without the if and switch had issues. Can you provide more info on what is supposed to happen in the GetCommand in your implementation, does the command actually varies with the parameters, as I am unsure what to suggest for that because of it.

eglasius
In the original code, the parsers acted as a non-polymorphic map and performed input validation on behalf of the command. The command (Utility) would pull values out of the parser. The new GetCommand may have to look at two+ parameters instead of just one to determine which Command to return.
James Schek
A: 

The third alternative would be to discover the possible commands at runtime in a decentralized way.

For example, Spring can do this in Java using so-called “classpath scanning”, reflection and annotations — Spring parses all classes in the package(s) you specify, picks ones annotated with @Controller, @Resource etc and registers them as beans.

Classpath scanning in Java relies on directory entries being added to JAR archives (so that Spring can enumerate the contents of various classpath directories).

I don't know about C#, but there should be a similar technique there: probably you can enumerate a list of classes in your assembly, and pick some of them based on some criteria (naming convention, annotation, whatever).

Now, this is just a third option to have in mind for the sake of having a third option in mind. I doubt it should actually be used in practise. You first alternative (just write a piece of code that knows about all the classes) should be the default choice unless you have a compelling reason to do otherwise.

In the decentralised world of OOP, where each class is a little piece of the puzzle, there has to be some “integration code” that knows how to put these pieces together. There's nothing wrong about having such “all-knowing” classes (as long as you limit them to application-level and subsystem-level integration code only).

Whichever way you choose (hard-code the possible choices in a class, read a config file or use reflection to discover the choices), it's all the same story, does not really matter, and can easily be changed at any time.

Have fun!

Andrey Tarantsov
I believe what you describe is called SmallTalk. :-)
James Schek
+2  A: 

You may want to put your parser instantiators on the objects themselves, e.g.,

public interface IParserInput
{
    ...
    IParser GetParser()
    ICommand GetCommand()
}

Any parameters that GetParser needs should, theoretically, be supplied by your object.

What will happen is that the object itself will return those, and what happens with your code is:

public ResultStruct HandleMessage(IParserInput input) 
{
    IParser parser = input.GetParser();
    Map<string,string> parameters = parser.Parse(input);
    ICommand command = input.GetCommand();
    return command.Execute(parameters);
}

Now this solution is not perfect. If you do not have access to the IParserInput objects, it might not work. But at least the responsibility of providing information on the proper handler now falls with the parsee, not the handler, which seems to be more correct at this point.

Jon Limjap
I think that's a cleaner approach. I have the refactor the inputs into an IParserInput anyway (they don't even inherit from a common interface right now)...
James Schek