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.
1 Answer 1
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.
-
\$\begingroup\$ I find it easier to understand the code if the
if
statement is before the loop. Otherwise I agree 100%. \$\endgroup\$2018年11月06日 18:37:08 +00:00Commented 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\$SkyeBoniwell– SkyeBoniwell2018年11月06日 18:58:48 +00:00Commented 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– Servy2018年11月06日 19:01:18 +00:00Commented Nov 6, 2018 at 19:01
Linq
? \$\endgroup\$foreach
is. \$\endgroup\$