Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TryDequeue to Queue #15619

Closed
benaadams opened this issue Nov 4, 2015 · 15 comments
Closed

Add TryDequeue to Queue #15619

benaadams opened this issue Nov 4, 2015 · 15 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Collections
Milestone

Comments

@benaadams
Copy link
Member

System.Collections.Generic.Queue does not have a TryDequeue method, however its Dequeue is a throwing method so the following pattern always has to be used:

T item ...;
if (queue.Count > 0)
{
    item = queue.Dequeue();
    ...
}

with extra boilerplate for for whatever no item handling; this would be better as:

T item;
if (queue.TryDequeue(out item)
{
    ...
}

As per ConcurrentQueue or TryGet of Dictionary

@zabulus
Copy link

zabulus commented Nov 4, 2015

You can write an extension method for this.

@joshfree
Copy link
Member

joshfree commented Nov 4, 2015

@benaadams do you want to try to flesh out the API proposal? Here's a link on the next steps http://aka.ms/apireview

@benaadams
Copy link
Member Author

@zabulus my point is unless you are doing exception based flow control; which would be terribad, everyone will always be writing it; extension or explicit, so it should be a part of the system library.

@s-sreenath
Copy link
Contributor

Is some one working on this already? If not I think I might be able to help with this.

@benaadams
Copy link
Member Author

@page-not-found go for it; should be fairly straightforward

@s-sreenath
Copy link
Contributor

@joshfree Can you help me on what needs to happen next? I suppose I need to show up some kind of code snippets which will be modified or something?
I read the link http://aka.ms/apireview but not sure i understand correctly.

@joshfree
Copy link
Member

@page-not-found here is a good example of a ready-to-review proposal

@s-sreenath
Copy link
Contributor

Thanks @joshfree I will start working on this week 👍

@s-sreenath
Copy link
Contributor

Problem Statement

As the above discussion points it out if we don't want to code for the exception flow if there is not item to the which can be dequeued instead of throwing an exception we should be returning default of T.

Proposed Changes

File: https://github.com/dotnet/corefx/blob/master/src/System.Collections/src/System/Collections/Generic/Queue.cs

/// <summary>
/// Attempts to remove and return the object at the beginning of the <see cref="Queue{T}"/>.
/// </summary>
/// <param name="result">
/// When this method returns, if the operation was successful, <paramref name="result"/> contains the
/// object removed. If no object was available to be removed, the value is unspecified.
/// </param>
/// <returns>true if an element was removed and returned from the beginning of the <see cref="Queue{T}"/>
/// successfully; otherwise, false.</returns>
public bool TryDequeue(out T result)
{
    if (_size == 0)
    {
        result = default(T);
        return false;
    }

    result = _array[_head];
    _array[_head] = default(T);
    MoveNext(ref _head);
    _size--;
    _version++;
    return true;
}

Usage Examples:

Copied from first comment

Before Change:

T item ...;
if (queue.Count > 0)
{
    item = queue.Dequeue();
    ...
}

After Change:

T item;
if (queue.TryDequeue(out item)
{
    ...
}

Proposed Tests

  1. EnqueueDequeueAndTryDeQueue
    • Queue few items and then try to use both Dequeue and TryDequeue for the same item
  2. TryDequeue
    • Try do dequeue an item with any item on the list.
  3. TryDequeAfterClear
    • Try do dequeue an item with any item on the list, after caling the clear method on the queue.

I don't see any open question so far.

Let me know if you thinnk there needs to more tests to be covered.

@joshfree
Copy link
Member

thanks @page-not-found and @benaadams. tagging this as api-ready-for-review.
/cc @terrajobst

@benaadams
Copy link
Member Author

As mentioned in #14032 TryPeek might be something to add at same time? Would follow similar pattern.

@ghost
Copy link

ghost commented Dec 21, 2015

LGTM 👍

@s-sreenath
Copy link
Contributor

@terrajobst Is there any update on this ?

@terrajobst
Copy link
Contributor

@page-not-found

Is there any update on this?

Sorry for the delay. We're currently focusing our efforts on getting to a good V1 release. This includes exposing more API that already exist in .NET Framework, which is what we focus our design meetings on. We don't spend much time on proposals to add APIs to existing types because even if were to approve them, the problem is that we don't have enough time left to add all these additions to the .NET Framework. Our goal for V1 is to have parity for the common APIs shared between .NET Core and .NET Framework. That's why this issue currently sits in milestone 'Future'.

@terrajobst
Copy link
Contributor

This looks useful. We should probably add

  • Queue<T>.TryDequeue()
  • Stack<T>.TryPeek()
  • Stack<T>.TryPop()

To make things consistent.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Collections
Projects
None yet
Development

No branches or pull requests

8 participants