HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Listing books by priority

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
prioritylistingbooks

Problem

```
#region GetBookListByPriority
private static List GetBookListByPriority(List listBcBook)
{
List newList = new List();
try
{
List listNonPriorityBcBooks = new List();
List listPriorityBcBooks = new List();

foreach (BCBook bcBook in listBcBook)
{
BookHistoryDto historyDto = new BookHistoryFacade().FindHistoryByBookIdByPriority(bcBook.id.ToString(CultureInfo.CurrentCulture));
if (historyDto != null && historyDto.HistoryId > 0)
{
bcBook.Priority = historyDto.Priority;
listPriorityBcBooks.Add(bcBook);
}
else
{
listNonPriorityBcBooks.Add(bcBook);
}
}
int count = 0;
int prio = 0;
foreach (BCBook bcBook in listNonPriorityBcBooks)
{
for (int j = count + 1; j listBook = new List();
listBook = ListBookPriority(listPriorityBcBooks, j, newList);
count++;
if (listBook.Count() > 0)
{
prio++;
}
else
{
break;
}
foreach (BCBook bc in listBook)
{
newList.Add(bc);
}
}
newList.Add(bcBook);
}
}
catch (Exception ex)
{
ErrorLogger.WriteErrorToLog(ex);
}

return newList;
}
#endregion

#region ListBookPriority
private static List ListBookPriority(List list, int priority, List newList)
{

List listBook = new List();
try
{
foreach (BCBook vid in list)
{
if (!IsExists(newList, vid.id.ToString(CultureInfo.CurrentCulture)))
{
if (vid.Priority == priority)
{
listBook.Add(vid);
}
}
}
}
catch (Exception ex)
{
ErrorLogger.W

Solution

I would get rid of foreach and use for. I have found (and read about) a huge performance difference between the 2 (EDIT, but not always, see below).

Example (untested code):

int c  = listBcBook.Count;

for (int i = 0; i  0)
    {
        bcBook.Priority = historyDto.Priority;
        listPriorityBcBooks.Add(bcBook);
    }
    else
    {
        listNonPriorityBcBooks.Add(bcBook);
    }
}


EDIT: I just read that the performance difference isn't always there because in some cases, the compiler will optimize your foreach loops into for loops. Here is a good read over on SO: https://stackoverflow.com/questions/1124753/for-vs-foreach-loop-in-c.

In your case, because you're using a List, directly using for should be faster.

Code Snippets

int c  = listBcBook.Count;

for (int i = 0; i < c; i++)
{
    BCBook bcBook = listBcBook[i];

    BookHistoryDto historyDto = new BookHistoryFacade().FindHistoryByBookIdByPriority(bcBook.id.ToString(CultureInfo.CurrentCulture));
    if (historyDto != null && historyDto.HistoryId > 0)
    {
        bcBook.Priority = historyDto.Priority;
        listPriorityBcBooks.Add(bcBook);
    }
    else
    {
        listNonPriorityBcBooks.Add(bcBook);
    }
}

Context

StackExchange Code Review Q#3315, answer score: 2

Revisions (0)

No revisions yet.