Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have 3 functions which look like this (simplified):

public void functionA(List list)
{
    // there is some functionA specific stuff here
    foreach (Object O in list)
    {
         if(O.a()) 
             functionD();
    }
}
public void functionB(List list)
{
    // there is some functionB specific stuff here
    foreach (Object O in list)
    {
         if(O.b()) 
             functionD();
    }
}
public void functionC(List list)
{
    // there is some functionC specific stuff here
    foreach (Object O in list)
    {
         if(O.c()) 
             functionD();
    }
}

It feels like I'm repeating myself too much, having three equal foreach loops with the same function call. I would like to move the loops to a separate function, but because the conditional is different each time, I don't know how.

Is it possible, and if so, how?

share|improve this question

closed as off-topic by abuzittin gillifirca, syb0rg, 200_success, Malachi, Donald.McLean Jan 13 at 16:03

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Your question must contain working code for us to review it here. For questions regarding specific problems encountered while coding, try Stack Overflow. After getting your code to work, you may edit this question seeking a review of your working code." – abuzittin gillifirca, syb0rg, 200_success, Malachi, Donald.McLean
If this question can be reworded to fit the rules in the help center, please edit the question.

3  
This wouldn't compile, because a isn't a method of Object. On this site people probably usually prefer working code, not pseudo-code. –  ChrisW Jan 12 at 22:59

5 Answers 5

up vote 10 down vote accepted

Use a parameterizable predicate:

public void function(List<T> list, Predicate<T> predicate)
{
    foreach (T O in list)
    {
         if(predicate(O)) 
             functionD();
    }
}

You can call it (create a predicate on-the-fly) using a lambda, like this:

public void functionB(List<T> list)
{
    function(list, O => O.b());
}
share|improve this answer

Agree with @ChrisW

And you can get rid all of them by using LINQ

foreach(var O in list.Where(O => O.b()))
{
    functionD();
}
share|improve this answer

I don't know C#, but this is how I would do it in Java. C# seems to have interfaces just like Java, so maybe this will be useful.

public interface Askable
{
    public boolean ask();
}

And then your functions would all be represented by one function of the form:

public void f(ArrayList<Askable> list)
{
    for (int i = 0; i < list.length; i++)
    {
        if (list[i].ask())
        {
             doStuff();
        }
    }
}
share|improve this answer

One way to make a function that encapsulates the foreach loop. Pass it a function object. Then you'd have three function objects with code like:

if (O.c()) functionD();

I can't remember my C# well enough - but maybe there is a way to extract out the specific member being called on the object O so that you can have a function which takes a list of objects, a member to call on each one and a function to call if that member returned true.

share|improve this answer

What language is that supposed to be?

  • List is generic in C# -> use generics
  • if(O.c()).... might work in some other languages; not so much in C#

Based on your "simplified" example, it's hard to give recommendations - I'd update the question with a more clear example: what's specific to each function?, where is the specific code located (after the if, in functionD()), etc...?

@ChrisW gave you an option; some other options here

share|improve this answer
    
A simple comment would have sufficed. –  user1895420 Jan 13 at 18:41

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