0
\$\begingroup\$

I have this code with nested foreach loops that is getting out of hand.

Is there a better and/or cleaner way of doing this?

Basically, what it's doing, is take the results from an API call, and then looping through each layer.

I check to make sure that each layers Results.Length is greater than zero before I start the next loop.

Anyway, here's my crazy ugly mess of code:

parentFolderResponse = GetFolders(parentFolder, sessionMgr, SessionManagementAuth, pagination);

        if (parentFolderResponse.Results.Length > 0)
        {
            foreach (Folder folder in parentFolderResponse.Results)
            {
                Console.WriteLine("\n + Class Of =  " + folder.Name);

                ListFoldersResponse childFolderResponse = GetFolders(folder.Id, sessionMgr, SessionManagementAuth, pagination);
                if (childFolderResponse.Results.Length > 0)
                {
                    foreach (Folder childFolder in childFolderResponse.Results)
                    {
                        Console.WriteLine("\n \t Course Name =  " + childFolder.Name);
                        ListSessionsResponse courseSessionResponse = GetPresentations(sessionMgr, SessionManagementAuth, pagination, childFolder.Id);
                        if (courseSessionResponse.Results.Length > 0)
                        {
                            foreach (Session session in courseSessionResponse.Results)
                            {
                                Console.WriteLine("n \t \t Session Name = " + session.Name);
                            }
                        } else {
                            Console.WriteLine("\n No sessions found!");
                        }
                    }
                } else {
                    Console.WriteLine("\n No course folder(s) found!");
                }

            }
        } else {
            Console.WriteLine("\n No class of folder(s) found!");
        }

If anyone has any suggestions, I'd love to hear them.

\$\endgroup\$
4
  • \$\begingroup\$ Heard about Linq? \$\endgroup\$
    – kara
    Nov 6, 2018 at 18:06
  • 1
    \$\begingroup\$ @kara This code isn't writing a query, it's iterating through sequences and performing an action on them. LINQ is not designed for that, foreach is. \$\endgroup\$
    – Servy
    Nov 6, 2018 at 18:11
  • \$\begingroup\$ My only advise is to turn this into a proper function. Translating a Bulk result to .NET classes sound like something you should make a function. Note that with weakly typed API's, there is a helper class: ExpandoObject. You can move the data to one of that instances, to then later translate it to proper .NET classes. \$\endgroup\$
    – Christopher
    Nov 6, 2018 at 18:14
  • \$\begingroup\$ @servy thanks, you're quite right of course. \$\endgroup\$
    – stuartd
    Nov 6, 2018 at 18:17

1 Answer 1

3
\$\begingroup\$

There's no reason to check if there are any items in a collection before iterating over it. If there are no items, iterating over it won't do anything. So that alone removes half of the indentation, which makes the code fairly manageable.

parentFolderResponse = GetFolders(parentFolder, sessionMgr, SessionManagementAuth, pagination);

foreach (Folder folder in parentFolderResponse.Results)
{
    Console.WriteLine("\n + Class Of =  " + folder.Name);

    ListFoldersResponse childFolderResponse = GetFolders(folder.Id, sessionMgr, SessionManagementAuth, pagination);
    foreach (Folder childFolder in childFolderResponse.Results)
    {
        Console.WriteLine("\n \t Course Name =  " + childFolder.Name);
        ListSessionsResponse courseSessionResponse = GetPresentations(sessionMgr, SessionManagementAuth, pagination, childFolder.Id);

        foreach (Session session in courseSessionResponse.Results)
        {
            Console.WriteLine("n \t \t Session Name = " + session.Name);
        }
        if (courseSessionResponse.Results.Length == 0)
        {
            Console.WriteLine("\n No sessions found!");
        }
    }
    if (childFolderResponse.Results.Length == 0)
    {
        Console.WriteLine("\n No course folder(s) found!");
    }
}
if (parentFolderResponse.Results.Length == 0)
{
    Console.WriteLine("\n No class of folder(s) found!");
}

You could also consider breaking the code up into several methods, to cover each of the logical objects being printed, rather than doing the whole thing all together. This is more useful if you're doing more stuff at each level, rather than just a conditional and unconditional console write at each tier, but it helps remove indenting, and is more useful the more tiers there are:

parentFolderResponse = GetFolders(parentFolder, sessionMgr, SessionManagementAuth, pagination);

foreach (Folder folder in parentFolderResponse.Results)
{
    PrintClass(folder);
}
if (parentFolderResponse.Results.Length == 0)
{
    Console.WriteLine("\n No class of folder(s) found!");
}

void PrintClass(Folder folder)
{
    Console.WriteLine("\n + Class Of =  " + folder.Name);

    ListFoldersResponse childFolderResponse = GetFolders(folder.Id, sessionMgr, SessionManagementAuth, pagination);
    foreach (Folder childFolder in childFolderResponse.Results)
    {
        PrintCourse(childFolder);
    }
    if (childFolderResponse.Results.Length == 0)
    {
        Console.WriteLine("\n No course folder(s) found!");
    }
}

void PrintCourse(Folder childFolder)
{
    Console.WriteLine("\n \t Course Name =  " + childFolder.Name);
    ListSessionsResponse courseSessionResponse = GetPresentations(sessionMgr, SessionManagementAuth, pagination, childFolder.Id);

    foreach (Session session in courseSessionResponse.Results)
    {
        Console.WriteLine("n \t \t Session Name = " + session.Name);
    }
    if (courseSessionResponse.Results.Length == 0)
    {
        Console.WriteLine("\n No sessions found!");
    }
}

Pulling out the printing of sessions seemed...not useful.

\$\endgroup\$
3
  • \$\begingroup\$ I find it easier to understand the code if the if statement is before the loop. Otherwise I agree 100%. \$\endgroup\$
    – Peilonrayz
    Nov 6, 2018 at 18:37
  • \$\begingroup\$ I'm sorry but what do you mean by, "Pulling out the printing of sessions seemed...not useful." ? Thanks! \$\endgroup\$ Nov 6, 2018 at 18:58
  • 1
    \$\begingroup\$ @SkyeBoniwell Writing a method to print a session, just as a method was created for printing courses and classes. \$\endgroup\$
    – Servy
    Nov 6, 2018 at 19:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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