Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a class called Task that contain a member boolean variable Completed.

I created a list of Task objects in a variable called Tasks.

I have the following code, which sorts the objects depending on whether the Task is completed. (I want the incompleted tasks to be in the list first)

private List<Task> GetSortedTask(List<Task> tasks)
{
    List<Task> completedTaskList = new List<Task>();
    List<Task> incompleteTaskList = new List<Task>();

    for (int i = 0; i < tasks.Count; i++)
    {
        HelperObject T = task[i].getHelperObject();

        if (T != null && T.Completed)
            completedTaskList.Add(tasks[i]);
        else
            incompletetaskList.Add(tasks[i]);
    }

    //merge the two lists together
    imcompleteTaskList.AddRange(completedTaskList);

    return incompleteTaskList;
}

Is there a way to do this without creating so many lists? Thanks in advance

EDIT: Made my question a bit more specific since orderBy seems to be the correct answer, however my problem may be a bit more complicated than that.

share|improve this question
1  
Just off the cuff, could you use tasks.OrderBy(p => p.Completed) –  dreza Nov 26 '12 at 20:42
    
@dreza I posted a vastly simplified version of the code, and I wasn't sure if OrderBy would work since it seemed like a Linq command and my class does not inhereit from IQueryable. But on second thought, it seems like it might work. I may have to edit my original question though EDITED question. –  Rhs Nov 26 '12 at 20:45
2  
It doesn't matter if your class doesn't inherit from IQueryable, so long as the data structure containing it (List) does. tasks.OrderBy(p => p.Completed) should be perfectly sufficient, unless you want the other direction, in which case you can use tasks.OrderByDescending(p => p.Completed). –  Bobson Nov 26 '12 at 22:15
    
@dreza,Bobson Why not post this as an answer? –  jpfollenius Nov 27 '12 at 7:12
    
@Smasher Yep, more answers the better if it adds something to the existing ones... –  dreza Nov 27 '12 at 7:34

3 Answers 3

up vote 7 down vote accepted

I think my original comment in regards to using OrderBy could still possibly still even with your latest edits (and BTW that code does not compile, there is no declared task object :)).

How about something like:

var query = from task in tasks
            let helperTask = task.getHelperObject()
            select new
            {
               Completed = helperTask != null && helperTask.Completed,
               Task = task
            };

return query.OrderBy(p => p.Completed).Select(p => p.Task).ToList();

UPDATE: Another excellent option suggested by svick to further refine and remove the anonymous object selection is something along the lines of:

var query = from task in tasks
            let helperTask = task.getHelperObject()
            orderby helperTask != null && helperTask.Completed
            select task;

return query.ToList();

I guess if I could I would do away with ToList() altogether and return IEnumerable but that depends on the context of the operation. I also personally like using an intermediate variable here (query) as I think it just helps readability. All to their own on that front though.

share|improve this answer
    
I'm unsure about your where clause. Wouldn't that filter out some of the tasks if their cooresponding helperTask is Null rather than treat them as "Not Completed"? –  Rhs Nov 26 '12 at 21:06
    
@Rhys modified the answer so that null helperTasks are not filtered now –  dreza Nov 26 '12 at 21:15
    
Thanks a lot for your help! –  Rhs Nov 26 '12 at 21:41
2  
I think you could simplify your code even more, you don't really need the anonymous object there: from task in tasks let helperTask = task.getHelperObject() orderby helperTask != null && helperTask.Completed select task. –  svick Nov 28 '12 at 17:43
    
@Svick excellent. Have updated the answer. –  dreza Nov 28 '12 at 19:11

I liked your original approach - it was fast. So, I changed it so that it now uses fewer lists.

private IEnumerable<Task> GetSortedTask(IList<Task> tasks)
{
    var incompleteTasks = new List<Task>();
    foreach (Task task in tasks)
    {
        var taskHelper = task.getHelperObject();
        if (taskHelper != null && taskHelper.Completed)
        {
            yield return task;
            continue;
        }

        incompleteTasks.Add(task);
    }

    foreach (Task task in incompleteTasks)
    {
        yield return task;
    }
}
share|improve this answer
    
Nice, I always forget about using yield return –  dreza Mar 11 '13 at 19:22

tasks.OrderBy(p => p.Completed) should do exactly what you're looking for, unless you want to sort in the other direction, in which case you can use tasks.OrderByDescending(p => p.Completed).

It doesn't matter if your class doesn't inherit from IQueryable, so long as the data structure containing it (List) does. That will enable you to use any LINQ functions via LINQ-to-Objects, because they're actually operating on the List<Task>, not a Task.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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