CODEWITHSUNDEEP
c#if-statementormsolid-principles
fhevol
fhevol

Reputation: 996

Avoiding multiple if statements in C# factory method

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.

Upvotes: 0

Views: 3410

Answers (4)

davidallyoung
davidallyoung

Reputation: 1332

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!

Upvotes: 0

B. Kemmer
B. Kemmer

Reputation: 1537

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.

Upvotes: 1

Coder55
Coder55

Reputation: 559

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

Upvotes: 1

Ralph
Ralph

Reputation: 3029

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)

Upvotes: 1

Related Questions