Search code examples
c#recursionside-effects

Recursion with the least sideeffect


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?


Solution

  • 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;
             }
        }
    }