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\$
  • \$\begingroup\$ Heard about Linq? \$\endgroup\$ – kara Nov 6 '18 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 '18 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 '18 at 18:14
  • \$\begingroup\$ @servy thanks, you're quite right of course. \$\endgroup\$ – stuartd Nov 6 '18 at 18:17
2
\$\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.

|improve this answer|||||
\$\endgroup\$
  • \$\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 '18 at 18:37
  • \$\begingroup\$ I'm sorry but what do you mean by, "Pulling out the printing of sessions seemed...not useful." ? Thanks! \$\endgroup\$ – SkyeBoniwell Nov 6 '18 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 '18 at 19:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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