patterncsharpMinor
PriorityQueue<T> implementation
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
```
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
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
This question on SO shows a basic implementation based on a SortedDictionary.
Technical
-
For consistency I would consider implementing all three
-
Consider changing
Same for
-
Also
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.