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

PriorityQueue<T> implementation

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

Problem

I was hoping to get some general feedback and thoughts about my implementation of a generic priority queue. Please feel free to be as critical as you want or see fit.

Generic priority queue implementation (please note that the PriorityConvention can be None, HighestPriorityInFront, LowestPriorityInFront):

```
using System;
using System.Collections;
using System.Collections.Generic;
using Utilities.Enums.Generics;
using Utilities.Interfaces;

namespace Utilities.Classes.Generics
{
/// A generic priority queue implementation.
/// David Venegoni, Jan 02 2014.
/// cref="T:System.Collections.Generic.IEnumerable{T}".
/// cref="T:Utilities.Interfaces.IPriorityQueue{T}".
/// Generic type parameter, where T
/// must implement the IComparable interface.
[Serializable]
public class PriorityQueue : IEnumerable, IPriorityQueue
where T : IComparable
{
#region Private Member Variables

private readonly List items; / The items in the queue /

#endregion

#region Properties

/// Gets the convention this priority queue uses to sort and insert items.
/// The ordering convention.
public PriorityConvention OrderingConvention { get; private set; }

/// Gets the number of items that are in the priority queue.
/// The number of items in the priority queue.
public Int32 Count
{
get { return items.Count; }
}

#endregion

#region Constructors

/// Initializes a new instance of the PriorityQueue class.
/// David Venegoni, Jan 02 2014.
/// Thrown when the convention is specified
/// as PriorityConvention.None.
///
/// (Optional) the convention to use when sorting and inserting items (this cannot be changed
/// after the priority queue is created).
///
pu

Solution

General

Priority queues in general keep a queue per priority. While your implementation achieves that, it is associated with a lot of copying when inserting and removing data due to the array backing structure of List. Using a SortedList with the priority as key and Queue as values would probably be faster and less code.

This question on SO shows a basic implementation based on a SortedDictionary.

Technical

-
For consistency I would consider implementing all three Clear methods with the same interface (returning the number of elements removed) and all go through one method. Something like this:

public int Clear()
{
    return Clear(0);
}

public int Clear(int startIndex)
{
    return Clear(startIndex, Count - 1 - startIndex);
}

public int Clear(int startIndex, int count)
{
    // Note: RemoveRange will throw if index and count are not a valid range within the list
    list.RemoveRange(startIndex, count);
    return count;
}


-
Consider changing PopFront to use an enumerator which avoids creating a temporary copy:

public IEnumerable PopFront(Int32 numberToPop)
{
    if (numberToPop > items.Count)
    {
        throw new ArgumentException(@"The numberToPop exceeds the number 
                                      of elements in the queue", "numberToPop");
    }

    while (numberToPop-- > 0)
    {
        yield return PopFront();
    }
}


Same for PopBack

-
FindIndexAndInsertItemAscending and FindIndexAndInsertItemDescending are almost exactly the same except for the PriorityConvention passed to FindIndexAndInsertItem so they should be refactored into one method.

Also List defines a BinarySearch method which should simplify your code somewhat. No need to reinvent the wheel.

Code Snippets

public int Clear()
{
    return Clear(0);
}

public int Clear(int startIndex)
{
    return Clear(startIndex, Count - 1 - startIndex);
}

public int Clear(int startIndex, int count)
{
    // Note: RemoveRange will throw if index and count are not a valid range within the list
    list.RemoveRange(startIndex, count);
    return count;
}
public IEnumerable<T> PopFront(Int32 numberToPop)
{
    if (numberToPop > items.Count)
    {
        throw new ArgumentException(@"The numberToPop exceeds the number 
                                      of elements in the queue", "numberToPop");
    }

    while (numberToPop-- > 0)
    {
        yield return PopFront();
    }
}

Context

StackExchange Code Review Q#38560, answer score: 8

Revisions (0)

No revisions yet.