3

I am trying to implement a priority queue mechanism using SortedDictionary and I would like to get suggestions on my current implementation.

My implementation is as follows:

public class PriorityQueue
{
    private Object lockObj;
    private SortedDictionary<PQMsgPriority, Queue<PQMessage>> messageDictionary; 

    public PriorityQueue()
    {
        lockObj = new object();
        messageDictionary = new SortedDictionary<PQMsgPriority, Queue<PQMessage>>();
    }

    public void Enqueue(PQMessage item)
    {
        lock (lockObj)
        {
            if(item != null && item.MsgPriority == PQMsgPriority.None)
            {
                if (messageDictionary.ContainsKey(item.MsgPriority))
                {
                    Queue<PQMessage> dataList = messageDictionary[item.MsgPriority];
                    dataList.Enqueue(item);
                    messageDictionary[item.MsgPriority] = dataList;
                }
                else
                {
                    Queue<PQMessage> dataList = new Queue<PQMessage>();
                    dataList.Enqueue(item);
                    messageDictionary.Add(item.MsgPriority, dataList);
                }
            }
        }
    }

    public PQMessage Dequeue()
    {
        lock (lockObj)
        {
            PQMessage messageData = null;
            PQMsgPriority deleteKey = PQMsgPriority.None;

            //If no data available, throw an exception
            if (messageDictionary.Count == 0)
                throw new InvalidOperationException();

            foreach (KeyValuePair<PQMsgPriority, Queue<PQMessage>> item in messageDictionary)
            {
                Queue<PQMessage> dataList = item.Value;
                messageData = dataList.Dequeue();
                messageDictionary[item.Key] = dataList;

                //If there is no more elements remaining in the list, set a flag (deleteKey) for deleting the key
                if (dataList.Count == 0) 
                    deleteKey = item.Key;

                break;
            }

            //If the deleteKey flag is set, delete the key from the dictionary
            if (deleteKey != PQMsgPriority.None)
                messageDictionary.Remove(deleteKey);

            return messageData;
        }
    }

    public int Count()
    {
        lock (lockObj)
        {
            return messageDictionary.Count;
        }
    }

    public PQMessage Peek()
    {
        lock (lockObj)
        {
            PQMessage messageData = null;

            //If no data available, throw an exception
            if (messageDictionary.Count == 0)
                throw new InvalidOperationException();

            foreach (KeyValuePair<PQMsgPriority, Queue<PQMessage>> item in messageDictionary)
            {
                Queue<PQMessage> dataList = item.Value;
                messageData = dataList.Peek();
                break;
            }

            return messageData;
        }
    }
}

public enum PQMsgPriority
{
    High = 0,
    Medium = 1,
    Low = 2,
    None = 3
}

public class PQMessage
{
    private PQMsgPriority msgPriority;
    private Object message;

    #region Properties
    public PQMsgPriority MsgPriority
    {
        get { return msgPriority; }
        set { msgPriority = value; }
    }
    public Object Message
    {
        get { return message; }
        set { message = value; }
    }
    #endregion

    public PQMessage(PQMsgPriority msgPriority, Object message)
    {
        this.msgPriority = msgPriority;
        this.message = message;
    }
}

If there are any other approach for implementing a priority queue, please do point me in the right direction.

Anil Mathew
  • 2,334
  • 1
  • 13
  • 13

2 Answers2

2

(I should just disclaimer this with I'm assuming you're doing this for learning rather than a bullet-proof implementation. If want something that just works, then you are better off reusing an existing implementation).

Just some general comments. In the example below, the third line is unnecessary.

Queue<PQMessage> dataList = messageDictionary[item.MsgPriority];
dataList.Enqueue(item);
messageDictionary[item.MsgPriority] = dataList;

The dataList you get returned from the messageDictionary is a copy of the reference within the map. This means that when you Enqueue the data, it's working on the same underlying queue as before (not a copy of the queue), therefore there's no need to put it back again, you can just remove that line.

In your dequeue implementation, you have a loop where you are breaking on the first element every time (e.g. you only ever go around the loop once). Perhaps you could investigate using LINQ to get the First element out and just return that straight away? (similarly for the Peek implementation`).

Finally, give that PQMessage has a priority itself, perhaps you could consider using a SortedList for your implementation? (see here)

Jeff Foster
  • 40,078
  • 10
  • 78
  • 103
1

If you needed to improve concurency performace you can lock only one queue for write. (Read lock for entire SortedDictionary still neaded).

And don't forget to set and release locks in right order

gabba
  • 2,735
  • 2
  • 21
  • 44