How to optimize the code below

Question

How can I optimize the below code, the below code contains an object array that might have different type of class objects. I would prefer to move to switch case instead of if...else... how could I do it.

object[] emailObjs;
for (int i = 0; i < emailObjs.Length; i++)
{
if (emailObjs[i] != null)
    {
       if (emailObjs[i] is Rfq)
       {

       }
       else if (emailObjs[i] is RfqBid)
       {

       }
       else if (emailObjs[i] is RfqProjManager)
       {

       }
    }
}

`Rfq`, `RfqBid` and `RfqProjManager` are related classes? Are they having inheritance relationship? — Chetan, Mar 27 '17 at 07:41
There is a new feature in C# 7.0: *Switch statements with patterns*, https://blogs.msdn.microsoft.com/dotnet/2016/08/24/whats-new-in-csharp-7-0/ . You could do `switch emailObjs[i] { case Rfq r: ... case RfqProjManager rpm: ... ` — xanatos, Mar 27 '17 at 07:41
If the order isn't important, you could use `foreach(var obj in emailObjs.OfType()) // do something` — Jeroen van Langen, Mar 27 '17 at 07:47
This is a duplicate of at least tens of other questions, what did your research show? Also, in general, when you need this, that's a design smell. What exactly is this code supposed to do? — CodeCaster, Mar 27 '17 at 07:47
@PawełSłowik: we are using VS2010 so no chance of C#7 pattern matching — Vasanthkani, Mar 27 '17 at 08:36

score 2 · Answer 1 · answered Mar 27 '17 at 07:47

Use C# 7 pattern matching in a switch statement with a type check:

switch (emailObjs[i])
{
    case Rfq r:
        {
            // use r;
            break;
        }
    case RfqBid r:
        {
            // use r;
            break;
        }
}

Although this will compact your code significantly, performance-wise it wouldn't have much benefit over an bunch of if statements, like in your sample code.

score 1 · Answer 2 · answered Mar 27 '17 at 07:49

1

Here is my try:

Rfq, RfqBid, and RfqProjManager implement the same interface. Let's call it IRfq.

interface
{
     void DoSomeWork();
}

Then, your for loop could be like this:

for (int i = 0; i < emailObjs.Length; i++)
{
    if (emailObjs[i] != null && emailObjs is IRfq)
    {
           ((IRfq)emailObjs[i]).DoSomeWork();
    }
}

Is it possible with your design ?

answered Mar 27 '17 at 07:49

Christophe

53
5

3

_"Is it possible with your design?"_ - then post a comment first to ask that, instead of guessing in an answer. – CodeCaster Mar 27 '17 at 07:49
Yes, I know, but that doesn't mean you should post answers instead... Anyway you now have 50 reputation. – CodeCaster Mar 27 '17 at 08:28
1

Thanks for the advice (and for the reputation). I will do it better next time. – Christophe Mar 27 '17 at 09:44

score 0 · Accepted Answer · answered Mar 27 '17 at 07:47

Define for all 3 classes common interface with one method:

public interface IRfq
{
    void DoSomething();
}

public class Rfq : IRfq
{
    public void DoSomething()
    {
        //...
    }
}

public class RfqBid : IRfq
{
    public void DoSomething()
    {
        //...
    }
}

public class RfqProjManager : IRfq
{
    public void DoSomething()
    {
        //...
    }
}

And you could do just one call in your for loop.

IRfq[] emailObjs;
for (int i = 0; i < emailObjs.Length; i++)
{
   emailObjs[i]?.DoSomething();
}

_If_ they want to call `DoSomething()` on all objects and it doesn't already have a base class defining that method. — CodeCaster, Mar 27 '17 at 07:47

How to optimize the code below

3 Answers3