patterncsharpMinor
Priority queue implementation in C#
Viewed 0 times
priorityimplementationqueue
Problem
My application has a thread for handling database inserts and updates. Data is added by other threads to a generic queue in the DB thread. The contents of this queue is retrieved and is sequentially inserted to the database by the thread.
To improve performance, I am planning to bring in a priority mechanism so that messages with higher priority should get inserted in the database before those with lower priority.
I implemented a class with a
Please review the following code and provide suggestions on how to improve it. Also if there are any readily available solutions please provide the details.
```
public class PriorityQueue
{
private Object lockObj;
private SortedDictionary> messageDictionary;
public PriorityQueue()
{
lockObj = new object();
messageDictionary = new SortedDictionary>();
}
public void Enqueue(PQMessage item)
{
lock (lockObj)
{
if (item != null)
{
if (messageDictionary.ContainsKey(item.MsgPriority))
{
Queue dataList = messageDictionary[item.MsgPriority];
dataList.Enqueue(item);
}
else
{
Queue dataList = new Queue();
dataList.Enqueue(item);
messageDictionary.Add(item.MsgPriority, dataList);
}
}
}
}
public PQMessage Dequeue()
{
lock (lockObj)
{
PQMessage messageData = null;
PQMsgPriority prioKeyDeleteFlag = PQMsgPriority.None;
//If no data available, throw an exception
if (messageDictionary.Count == 0)
throw new InvalidOperationException();
foreach (KeyValuePair> item in m
To improve performance, I am planning to bring in a priority mechanism so that messages with higher priority should get inserted in the database before those with lower priority.
I implemented a class with a
SortedDictionary for handling the priority. An enum indicating the priority would be the key and a Queue containing the messages would be the value part of the dictionary.Please review the following code and provide suggestions on how to improve it. Also if there are any readily available solutions please provide the details.
```
public class PriorityQueue
{
private Object lockObj;
private SortedDictionary> messageDictionary;
public PriorityQueue()
{
lockObj = new object();
messageDictionary = new SortedDictionary>();
}
public void Enqueue(PQMessage item)
{
lock (lockObj)
{
if (item != null)
{
if (messageDictionary.ContainsKey(item.MsgPriority))
{
Queue dataList = messageDictionary[item.MsgPriority];
dataList.Enqueue(item);
}
else
{
Queue dataList = new Queue();
dataList.Enqueue(item);
messageDictionary.Add(item.MsgPriority, dataList);
}
}
}
}
public PQMessage Dequeue()
{
lock (lockObj)
{
PQMessage messageData = null;
PQMsgPriority prioKeyDeleteFlag = PQMsgPriority.None;
//If no data available, throw an exception
if (messageDictionary.Count == 0)
throw new InvalidOperationException();
foreach (KeyValuePair> item in m
Solution
- I don't like that you're treat
PQMsgPriority.Nonein a special way inDequeue(). If somebody tried to enqueue something with this priority, the code wouldn't work as expected.
- I think your
Peek()wouldn't work correctly in some cases. Specifically,Dequeue()can leave the queue with the highest priority empty, while other queues still have items. In this casePeek()would incorrectly returnnull.
- I think you're overusing
breakandcontinue. They can be useful, but I think the way you're using them makes your code harder to read. For example your (incorrect) code inPeek()could be simply rewritten usingFirst().
- Your
Count()could be simplified tomessageDictionary.Values.Sum(q => q.Count). Especially the check for0seems completely useless here (possibly an attempt at nano-optimization?). Thenullcheck also seems useless, if the dictionary containednullvalues, that's a bug you're hiding by this.
- It would make sense to make this class generic in both the priority type and the message type.
Context
StackExchange Code Review Q#11836, answer score: 8
Revisions (0)
No revisions yet.