noamyogev84 / ConcurrentPriorityQueue

A thread-safe generic first in-first out (FIFO) collection with support for priority queuing.
MIT License
22 stars 6 forks source link

Check Queue.Count > 0 before call into Queue.Peek() #18

Closed zhuxb711 closed 1 year ago

zhuxb711 commented 1 year ago

Thanks for your great work. But I have some suggestion.

It would be great if we could check if queue.Count > 0 before call queue.Peek() or just use queue.TryPeek(). If queue is empty, then queue.Peek() will threw exception, which will affect the performance deeply. Thanks.

https://github.com/noamyogev84/ConcurrentPriorityQueue/blob/e512c33078d336904aee42e83b45630df3165081/ConcurrentPriorityQueue/Core/ConcurrentPriorityQueue.cs#L103

Solution 1:

        private Result<Queue<T>> GetNextQueue()
        {
            foreach (var queue in _internalQueues.OrderBy(q => q.Key).Select(q => q.Value))
            {
               if(queue.Count > 0){
                    return Result.Ok(queue);
               }
            }

            return Result.Fail<Queue<T>>("Could not find a queue with items.");
        }

Solution 2:

        private Result<Queue<T>> GetNextQueue()
        {
            foreach (var queue in _internalQueues.OrderBy(q => q.Key).Select(q => q.Value))
            {
               if(queue.TryPeek(out _)){
                    return Result.Ok(queue);
               }
            }

            return Result.Fail<Queue<T>>("Could not find a queue with items.");
        }

Solution 3:

        private Result<Queue<T>> GetNextQueue()
        {
            if (_internalQueues.OrderBy(q => q.Key).Select(q => q.Value).FirstOrDefault(q => q.Count > 0) is Queue<T> queue)
            {
                return Result.Ok(queue);
            }

            return Result.Fail<Queue<T>>("Could not find a queue with items.");
        }
github-actions[bot] commented 1 year ago

Hey there, freshman! Wassup?

zhuxb711 commented 1 year ago

@noamyogev84 Hope that would help your repo, thanks!

noamyogev84 commented 1 year ago

Hey there @zhuxb711 I like your suggestion! Feel free to issue A PR

zhuxb711 commented 1 year ago

Would you mind let me help you upgrade the whole repo and libraries to the latest?

zhuxb711 commented 1 year ago

Please check #19