Avoid repeating for loop for different code execution

Question

I have a condition let's assume for example Animal = {Dog,Cat,Elephant}

Now I want to make a for loop with if conditions in it (not a simple for loop), inside this for loop I do some code based on the animal type for example:

for(int i=0;i<100;i++)
{
  if(some conditions on i)
  {
   for(int j =0;j<100;j++)
   {
     if(some condition on j)
     {
       switch(animaltype)
       {  
         case Dog: //call function 1
         case Cat: //call function 2
         case Elephant: //call function 3
       }
     }
   }
  }
}

So for performance optimization in case of large loops I made the switch-case outside the for loop so the code became something like this:

switch (animaltype)
{
case Dog :
    for(int i=0;i<100;i++)
    {
      if(some conditions on i)
      {
       for(int j =0;j<100;j++)
       {
         if(some condition on j)
         {
           //call function 1
         }
       }
      }  
    }
//-------------
case Cat :
    for(int i=0;i<100;i++)
    {
      if(some conditions on i)
      {
       for(int j =0;j<100;j++)
       {
         if(some condition on j)
         {
           //call function 2
         }
       }
      }  
    }
//----------------------
case Elephant :
    for(int i=0;i<100;i++)
    {
      if(some conditions on i)
      {
       for(int j =0;j<100;j++)
       {
         if(some condition on j)
         {
           //call function 3
         }
       }
      }  
    }
}

The problem here is that I repeated the code 3 times (or as the number of cases) and this violates the once and only once principle of the Software Design.

I tried to pass a delegate but the 3 functions that I supposed to call have different arguments, can anyone tell me a neat solution to this case?

EDIT I mean by "different arguments" that they do not take the same number of arguments. For example:

function1(string s,int age)
function2(float height,bool CanMove)
function3(int legs,string name,string place)

That's the reason why methods exist. Also, don't worry about the performance of a switch. — Tim Schmelter
– Tim Schmelter, Commented Jun 5, 2014 at 13:08
It would help if you'd give a more complete example. The key will lie in the differences in terms of what you need to do with the animal... (I strongly suspect a delegate is the right solution here, but we can't give a more complete example of the solution without a complete example of the problem...) — Jon Skeet
– Jon Skeet, Commented Jun 5, 2014 at 13:09
Another thing is that you should never prematurely optimize your code. I don't believe you've actually made any performance improvements by rearranging the execution like that. If anything, you've only worsened the readability. — Will Custode
– Will Custode, Commented Jun 5, 2014 at 13:12
a switch statement is not going to be the performance problem here, it turns into a simple jmp in asm. — Kevin Cook
– Kevin Cook, Commented Jun 5, 2014 at 13:14
Also consider whether the animal should be a class rather than an enum (or string or whatever it is here), with the function as a method on that class. Without knowing what exactly the method is, it's hard to say whether that's the case here — Ben Aaronson
– Ben Aaronson, Commented Jun 5, 2014 at 13:21

Will Custode · Accepted Answer · 2014-06-05 13:25:09Z

2

Try something like this:

void ExecuteLoop(Func callback)
{
    for(int i=0;i<100;i++)
    {
        if(some conditions on i)
        {
            for(int j =0;j<100;j++)
            {
                if(some condition on j)
                {
                    callback();
                }
            }
        }  
    }
}

switch (animaltype)
{
case Dog:
    ExecuteLoop(dogCallback);
    break;
case Cat:
    ExecuteLoop(catCallback);
    break;
case Elephant:
    ExecuteLoop(elephantCallback);
    break;
}

This will allow you to consolidate the loop into a method, while varying what is actually executed. It is unclear when you say:

...but the 3 functions that I supposed to call have different arguments...

what you mean. I assume one of two things:

The three methods you intend to call have different values passed as their arguments.

or

The three methods you intend to call have a different number of arguments passed to them.

Either way, you can solve this by building on the previous solution. Something like this:

switch (animaltype)
{
case Dog:
    ExecuteLoop(() => { dogCallback(1, 2, 3); });
    break;
case Cat:
    ExecuteLoop(() = > { catCallback( "argument 1", "arg 2" ); });
    break;
case Elephant:
    ExecuteLoop(() => { elephantCallback(i, j, k); });
    break;
}

This uses lambdas that accept no parameters (thus matching the requirement of the ExecuteLoop method) but call a method that accepts any number of variable type arguments.

edited Jun 5, 2014 at 13:25

answered Jun 5, 2014 at 13:19

Will Custode

4,6343 gold badges28 silver badges52 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Ben Aaronson Over a year ago

Given the amount of information we have from the question, this is probably the best solution.

Robert Levy · Accepted Answer · 2014-06-05 13:14:06Z

1

I tried to pass a delegate but the 3 functions that I supposed to call have different arguments, can anyone tell me a neat solution to this case?

Create 3 functions which take no parameters which call your existing functions. For example...

Func HandleDog = ()=>{function1(param1, param2);};
Func HandleCat = ()=>{function2(param1);};

answered Jun 5, 2014 at 13:14

Robert Levy

29.1k6 gold badges66 silver badges96 bronze badges

Comments

Philip Pittle · Accepted Answer · 2014-06-05 13:25:58Z

1

To expand on the answers provided by Robert and William, I personally find this cleaner:

        Action animalMethod;

        switch (animalType)
        {
            case Dog:
                animalMethod = new Action(() => CallMethod1(animal as Dog));
                break;
            case Cat:
                animalMethod = new Action(() => CallMethod1(animal as Dog));
                break;
            case Elephant:
                animalMethod = new Action(() => CallMethod1(animal as Dog));
                break;
            default:
                throw new Exception("Unknown Animal");
        }

        for (var i = 0; i < 100; i++)
        {
            if (some conditions on i)
            {
                for (var j = 0; j < 100; j++)
                {
                    if (some condition on j)
                    {
                        animalMethod();
                    }
                }
            }
        }

answered Jun 5, 2014 at 13:25

Philip Pittle

12.5k8 gold badges69 silver badges127 bronze badges

2 Comments

Will Custode Over a year ago

For what it's worth, our solutions are exactly the same number of lines of code: 28. Just a fun fact!

Philip Pittle Over a year ago

The difference though was I kept the for loop in the same method that assigns the call back delegate. Also, I handle the default case in the switch statement and throw a very nicely formatted exception :). But yes, it is mostly stylistic. The performance overhead of an extra method call would be negligible.

Collectives™ on Stack Overflow

Avoid repeating for loop for different code execution

3 Answers 3

1 Comment

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related