0

I am relatively new to recursion. I am running into a weird situation. As you can see below I am populating employees for a complex department. A complex department can have one or more simple departments and complex departments under it. It also has a collection (list) of employees in it.When I debug through this the first department is complex so it does recursion and in there I see that the employees are getting correctly populated and the employees collection count is 2, but when it comes out of the recursive call the employees collection count is again set to zero. Any ideas as to what I might be doing wrong here?

private void PopulateEmployees(ComplexDepartment complex)
{            
    foreach (var dep in complex.Departments)
    {                
        if (dep is SimpleDepartment)
        {
             var simple = dep as SimpleDepartment;
             complex.employees.Add(GetEmployee(simple));
        }
        else if (dep is ComplexDepartment) 
        {
            PopulateEmployees(dep as ComplexDepartment);
        }                
    }            
}

private Employee GetEmployee(SimpleDapartment simple)
{
    var employee = new Employee();
    // some code here

    return employee;
}
vgru
  • 49,838
  • 16
  • 120
  • 201
user1527762
  • 927
  • 4
  • 13
  • 29
  • when you recurse in your `else if` branch you will add the additional employees to the collection in `department` ... – Random Dev Aug 10 '14 at 13:16
  • 1
    You are populating employees for a child complex department. When you get back from the recursive call you're looking at the parent department, where you didn't actually add anything directly. – vgru Aug 10 '14 at 13:41

1 Answers1

1

To get your code working (quick and dirty):

private void PopulateEmployees(ComplexDepartment complex)
{            
    foreach (var dep in complex.Departments)
    {                
        if (dep is SimpleDepartment)
        {
             var simple = dep as SimpleDepartment;
             complex.employees.Add(GetEmployee(simple));
        }
        else if (dep is ComplexDepartment) 
        {
            PopulateEmployees(dep as ComplexDepartment);
            // you need this here, because you collect the additional employees in dep
            complex.employees.AddRange(dep.employees);
        }                
    }            
}

To get your code working (cleaner):

private void PopulateEmployees(ComplexDepartment complex, ComplexDepartment addTo)
{            
    foreach (var dep in complex.Departments)
    {                
        if (dep is SimpleDepartment)
        {
             var simple = dep as SimpleDepartment;
             addTo.employees.Add(GetEmployee(simple));
        }
        else if (dep is ComplexDepartment) 
        {
            PopulateEmployees(dep as ComplexDepartment, addTo);
        }                
    }            
}

// Call it like
PopulateEmployees(myDep, myDep);

Why not do it non-destructive:

   private IEnumerable<Employee> EnumerateEmployees(ComplexDepartment complexDepartment)
    {            
        foreach (var department in complexDepartment.Departments)
        {                
            if (department is SimpleDepartment)
                yield return GetEmployee((SimpleDepartment) department);
            else if (department is ComplexDepartment) 
            {
                foreach(var e in EnumerateEmployees(department))
                   yield return e;
            }                
        }            
    }

But not that you might get problems with StackOverflows if your structure is very deep - so I would suggest doing this with an accumulator or just using LINQ.

The code is just to demonstrate how you could do it using recursion ;)

Random Dev
  • 51,810
  • 9
  • 92
  • 119
  • A simple department can have only one employee so the GetEmployee method returns only one employee. – user1527762 Aug 10 '14 at 13:25
  • a yes sorry - easily fixed ;) – Random Dev Aug 10 '14 at 13:30
  • Thanks. But I still don't understand what wrong I am doing in my code that the employees collection is getting cleared when it comes out of recursion? – user1527762 Aug 10 '14 at 13:37
  • you add the employes to the departments you iterate over - I add a section of how you could fix this – Random Dev Aug 10 '14 at 13:38
  • just came across this: http://stackoverflow.com/questions/3969963/when-not-to-use-yield-return Will yield return will be good for my code since it uses recursion? – user1527762 Aug 10 '14 at 13:48
  • the warning there applies here yes - but is this really an issue? Are we talking about a real world example where there surely aren't that many levels of "Departments" or are we talking theoretical limits (like thousands of levels deep)? – Random Dev Aug 10 '14 at 14:07