3

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.

  • 5
    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
  • 1
    possible duplicate of Approaches to checking multiple conditions? – GlenH7 Jun 18 '14 at 14:20
  • 2
    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! – Rob Jun 18 '14 at 14:23
  • 1
    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
  • 3
    @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 8:23
4

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 7:19
  • @marianoc84 I've updated the code to reflect the edit you made to the question. – Kevin Hogg Jun 19 '14 at 7: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 Process2are not derived from Process any more. – Doc Brown Jun 19 '14 at 8: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 8: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
0

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.