Reduce number of if statements through design pattern

Question

I had a loop through object Process, each process instance can be of a different type, derived from Process base class (e.g.: Process1, Process2,...). Each derived type of Process has different properties. For instance: some Processes are indexable, this is notified by IsIndexable flag. When a Process is indexable has some additional properties (for instance: AccessDate) that non-indexable process doesn't have. Now I have to cycle on each Process in request.Process (remember indexable Processes are different from others)

foreach (Process process in request.Processes)
{
     if(process.getType().Name.Equals("Process1")) // indexable process
     {
         ((Process1)process).Name = "aName";
         ((Process1)process).AccessDate = DateTime.Now;
     }
     else if(process.getType().Name.Equals("Process2")) // non indexable process
     {
         ((Process2)process).Name = "anotherNane";
         //compile error - AccessDate don't exist for type Process2
         //((Process2)process).AccessDate = DateTime.Now;          
     }
}

Since I hate that cascading if I have rewritten using interface:

IProcessable processableImpl = // some Unity stuff based on request type
foreach (Process process in request.Processes)
{
     processableImpl.fillTheRightProperties(process);
}

processableImpl is injected in a different manner based on the request.Type. At this point fillTherRightProperties method will do the work for me on the current process.

public interface IProcessable
{
    void fillTheRightProperties(Process process);
}

public class IndexableProcess : IProcessable 
{
    void fillTheRightProperties(Process process){
        Process1 process1 = process as Process1;

        if(process1==null) throw MyException("Process1 expected");

        process1.Name = "aName";
        process1.AccessDate = DateTime.Now;
    }
}

public class NonIndexableProcess : IProcessable 
{
    void fillTheRightProperties(Process process){
        Process2 process2 = process as Process2;

        if(process2==null) throw MyException("Process2 expected");

        process2.Name = "aName";
    }
}

This is more beautiful than a cascading if but I feel still not as beautiful as it could be. I feel a violation of responsability, since the concrete class edit process property elsewhere, and I'm afraid to read this code a week after.

Doesn't your `Process` provide a method `doSomething()` that's being implemented by the deriving classes? If so (and it probably should be so), you could just call `process.doSomething()` without the need to downcast... Could you clarify by showing some `Process` code? — jhr, Jun 18 '14 at 14:09
possible duplicate of [Approaches to checking multiple conditions?](http://programmers.stackexchange.com/questions/191208/approaches-to-checking-multiple-conditions) — , Jun 18 '14 at 14:20
Your more elegant solution is polymorphism (i.e. accessing a bunch of different types through a common interface). In OO, it's the way you're supposed to do that -- so thumbs up! — sea-rob, Jun 18 '14 at 14:23
And while we're at it... Your `Process2` might be a subclass of `Process1`. What should be executed now, i.e. is the type's name relevant or are you rather looking for `if (process is Process1)` or something like that? — jhr, Jun 18 '14 at 14:26
Also, your second loop doesn't actually do anything with the loop variable `process`. It's not quite clear what your intentions there are. — FrustratedWithFormsDesigner, Jun 18 '14 at 14:33
@FrustratedWithFormsDesigner edited. Frustrations make me write terrible post. — BAD_SEED, Jun 19 '14 at 07:46
@marianoc84 your recent edits introduce a dependency on the `Process` class which interfaces prevent. Your code is more "highly coupled" than before which is a bad practice. See my updated answer for a more "loosely coupled" approach. — Kevin Hogg, Jun 19 '14 at 08:00
@GlenH7: this is not a duplicate of the question you linked to (at least, after the edit), you may consider to retract your closing vote? — Doc Brown, Jun 19 '14 at 08:23

Kevin Hogg · Accepted Answer · 2014-06-19T07:57:54.033

Below is an example using an interface and two implementations in a console application:

using System;
using System.Collections.Generic;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var processes = new List<IProcessable>();
            processes.Add(new Process1());
            processes.Add(new Process2());

            foreach (IProcessable item in processes)
            {
                item.FillTheRightProperties();
            }

            Console.WriteLine();
            Console.WriteLine("Press Enter/Return to exit...");
            Console.ReadLine();
        }
    }

    interface IProcessable
    {
        void FillTheRightProperties();
    }

    class Process1 : IProcessable
    {
        public string Name { get; set; }
        public DateTime AccessDate { get; set; }

        public void FillTheRightProperties()
        {
            this.Name = "aName";
            this.AccessDate = DateTime.Now;

            Console.WriteLine("Properties filled: {0}, {1}", this.Name, this.AccessDate);
        }
    }

    class Process2 : IProcessable
    {
        public string Name { get; set; }

        public void FillTheRightProperties()
        {
            this.Name = "aName";

            Console.WriteLine("Properties filled: {0}", this.Name);
        }
    }
}

The key line is in the foreach where we use the interface rather than casting to a concrete class.

To map this to your example request.Processes would be a List<IProcessItem> meaning we can guarantee that the DoSomething() method exists, and there is no need to cast to a concrete class.

The base class can also be used, but it's purpose would be to hold common code only.

I can't use a IProcessItem like you suggest, i've implemented things a little bit different... — BAD_SEED, Jun 19 '14 at 07:19
@marianoc84 I've updated the code to reflect the edit you made to the question. — Kevin Hogg, Jun 19 '14 at 07:58
@KevinHogg: I think you can improve your posting to reflect the situation of the OP even better. Replace the `interface IProcessable` by an abstract class derived from `Process` and make `FillTheRightProperties` a pure virtual method of that class. In your current implementation, `Process1` and `Process2`are not derived from `Process` any more. — Doc Brown, Jun 19 '14 at 08:30
1) I can't edit in this way, considere Process like IProcessable will produce side effect on the entire platform, since Process is a domain class shared by everyone. Surely we should do this refactoring. 2) What about responsability issue? I still think that in this way we handle process properties in the wrong place, far from Process object. Am I just paranoid? — BAD_SEED, Jun 19 '14 at 08:42
1) Within the context of the code sample using `IProcessable` would not impact other users as it is limited to within the `foreach` loop specified above; if you had to *return* a concrete class, then we couldn't say this. 2) Using the interface allows responsibility to remain with the concrete classes, not the calling method (i.e. within the `foreach` loop); this approach reduces responsibility through polymorphism. — Kevin Hogg, Jun 19 '14 at 11:12

score 0 · Answer 2 · answered Jun 18 '14 at 14:13

Use a hash which contains as key the name of the Process and as value the Object containing the process. That way you get just one line of code:

Note: the following is in perl since I don't know C#, but the logic is the same

$hashContainingProcesses{processName}.doSomething();

Depending on how C# will handle this you probably want to check first if the key exsists.

Reduce number of if statements through design pattern

2 Answers2