Avoiding multiple if statements in C# factory method

Question

I have C# application that stores data in a NoSQL database. The app has a repository class that uses a series of objects to translate the data from the NoSQL form to that used by the C# models, and vice versa (essentially a form of ORM). The objects implement a converter interface, and each object works on one specific data type (string, bool, etc). Internally they use reflection to perform the transformation. Instances of these objects are created by a method that returns the object for the given type. Currently the logic looks something like the following:

if(type == typeof(string)
    return new StringConverter(...);

if(type == typeof(int) || type == typeof(uint))
    return new IntegerConverter(...);

... // and so on

However all those if statements bother me. I know I could do something like create a dictionary to map types to creation methods but I am not sure if this will result in more readable, easy to maintain/extend code(?). Given the need to create the type abstractions what is the best way to go about doing this? Any suggestions welcomed. Huge thanks in advance.

Do IntegerConverter and StringConverter has a basetype? If so you can use this solution - http://stackoverflow.com/a/5847468/442444 — Carbine, Jul 30 '15 at 06:48

score 1 · Accepted Answer · answered Jul 30 '15 at 06:31

1

Ideally, nested if-statements are what you want to avoid. I am quoting Microsoft's Complete Code book. The idea is when you nest up to 3 levels, the attention of the developer maintaining the code or yours as the developer decreases drastically. In your case, if your logic has to be hardcoded, there is no way around having a sequence of if-statements. However a smart way around it would be to follow the Factory design pattern.(Gangs of Four)

answered Jul 30 '15 at 06:31

Ralph

2,959
9
26
49

Thanks for your comments, much appreciated. I am already using a factory to perform the actual creation so may just leave things as they are. – fhevol Jul 30 '15 at 20:55

score 1 · Answer 2 · answered Jul 30 '15 at 06:36

1

You should export your if-code to a factory class.

As you can see here, the factory is a GoF pattern which produces different kind of objects.

In a factory class, this amount of int cases is okay.

You should only avoid nested if statements, they make your code unreadable

answered Jul 30 '15 at 06:36

Coder55

559
5
17

Big thanks for your comments, it really is great how people are prepared to help around here. I am already using a factory to create the actual objects. I omitted this detail for the sake of brevity in my initial query. – fhevol Jul 30 '15 at 20:57

score 1 · Answer 3 · edited May 23 '17 at 11:51

In terms of readability, you should prefer

switch(type.FullName){
  case(typeof(int).FullName) => //your logic
  ...
}

A switch-case-Statement is way more readable Code than a If-Else and can be faster. HERE you can read more about it:

For just a few items, the difference is small. If you have many items you should definitely use a switch.

If a switch contains more than five items, it's implemented using a lookup table or a hash list. This means that all items get the same access time, compared to a list of if:s where the last item takes much more time to reach as it has to evaluate every previous condition first.

And as everyone else already said, outsource your switch-case into a factory Class.

Thanks for your comments, I really appreciate your input. As noted above I am already using a factory to perform the actual creation. I will look at the switch statement, however some of the cases use logic like type.IsEnum, so this may not be straight forward. — fhevol, Jul 30 '15 at 20:56

score 0 · Answer 4 · answered Dec 09 '15 at 15:51

I believe your idea on mapping the types to a dictionary would be your best bet, as this implements a Strategy pattern and is pretty expressive in and of itself.

var converterStrategies = new Dictionary<Object, IConverter>();
converterStrategies.Add(typeOf(string), new StringConverter(...));

Then use a TryGetValue while passing in your referenced type.

You may would consider having this dictionary be a private member of the class and have it populated on initialization of the containing class.

Gary McLean Hall demonstrates this pattern in his book Adaptive Code via C#, it has helped me a great deal in situations like this!

Avoiding multiple if statements in C# factory method

4 Answers4