Recursion with the least sideeffect

72 views Asked by At

So I have a tree of people with children and I only want to get the people with cars. if a child has a car but the parent does not, I want to keep the parent in the tree.

I thought the best way would be to use a recursive function, that looked like this:

private Person CheckPerson(Person person)
{
    List<Person> removeList = new List<Person>();

    foreach (Person child in Person.Children)
    {
        if (CheckPerson(child) == null)
        {
            // I can't remove the children here because
            // they are used in the foreach loop
            removeList.Add(child);
        }

    }

    foreach (Person removable in removeList)
    {
        Person.Children.Remove(removable);
    }

    if (person.Children.Count() > 0)
    {
        return person;
    }
    else if (person.cars.Count() > 0)
    {
        return person;
    }
    else
    {
        return null;
    }
}

but this way I do change the parameter person, I remove his children.

The other ways I tried are as below.

  • return void
  • return bool
  • ref parameter

I also tried doing it without a return value and just looking back at the input person for the result, but this didn't save the changes.

Returning a bool to determine if a child should be removed worked, but the original method call did not have any use for the bool. So the method had side effects since Person -> bool, but I still changed the person

using a ref parameter I wasn't able to compile because I remove the children and there for changing the object in my foreach loop.

So I'm wondering what the best way would be to use a recursive method with the least side effects, or what the best practice would be?

1

There are 1 answers

3
Sebastian Schumann On BEST ANSWER

If you only need a Test whether a car is available you could use this:

public Person CheckPerson(Person person)
{
    return person.Cars.Any() || person.Children.Any(x => CheckPerson(x) != null) ? person : null;                    
}

But I think you should return an IEnumerable<Person> that contains all persons that have a car (or one child has a car).

Maybe is could look like this:

public IEnumerable<Person> GetPersonsWithCars(Person person)
{
    var personReturned = false;
    if (person.Cars.Any())
    {
        yield return person;
        personReturned = true;
    }

    foreach (var child in person.Children)
    {
         foreach (var item in GetPersonsWithCars(child))
         {
             if (!personReturned)
             {
                 yield return person;
                 personReturned = true;
             }

             yield return item;
         }
    }
}