HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

A queue with access to the last element

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastthewithelementqueueaccess

Problem

Following this question, I realized there is no built-in data structure like that, so I implement my own queue using List.

Please comment on performance (is there any better way instead of using List?)

public class MyQueue
{
    private List _items;
    private int index;

    public int Count
    {
        get
        {
            return _items.Count;
        }
    }

    public T Last
    {
        get
        {
            return _items[index];
        }
    }

    public MyQueue()
    {
        index = -1;
        _items = new List();
    }

    public void Enqueue(T item)
    {
        _items.Add(item);
        index++;
    }

    public T Dequeue()
    {
        T i = _items[0];
        _items.RemoveAt(0);
        index--;
        return i;
    }

    public T Peek()
    {
        return _items[0];
    }
}

Solution

In general, when trying to reimplement basic collections like this, you're going to have a lot of difficulty getting near the performance of the in-built .NET collections. As others have said, removing frequently from the beginning of a List has very poor performance. However, just looking at the circular buffer article already linked, my immediate reaction is "yeesh, that is not complexity I'd want to add to my code!" Especially when I know somebody's already done it for me in the existing Queue!

So in situations like this, the answer is usually to either be willing to sacrifice performance, or to try to make use of the actual collection as much as possible. In fact, all you want is a queue that has a little extra functionality, so why not do just that, using composition? There's no IQueue interface in .NET, so we can't implement the decorator pattern, but we can get close:

class MyQueue
{
    private readonly Queue _inner = new Queue();

    public int Count
    {
        get
        {
            return _inner.Count;
        }
    }

    public T Last { get; private set; }

    public void Enqueue(T item)
    {
        _inner.Enqueue(item);
        Last = item;
    }
    //...
}


In more general review terms, the code you wrote looks good. A few minor points:

  • index and _items follow different naming schemes.



  • MyQueue is not a good class name. Try to pick a name that actually describes the class



  • As firda said, you shouldn't try to keep track of the index yourself, it's just Count - 1. You're tracking the same thing in two different ways.



  • You may want to consider implementing IEnumerable. Using iterator methods, this wouldn't be too complicated, and it'd allow the full power of LINQ to be used with your class. However, I would definitely file this under YAGNI until it actually comes up as something you find yourself wanting, unless your intention with this code is to put it in a public library for others to use.

Code Snippets

class MyQueue<T>
{
    private readonly Queue<T> _inner = new Queue<T>();

    public int Count
    {
        get
        {
            return _inner.Count;
        }
    }

    public T Last { get; private set; }

    public void Enqueue(T item)
    {
        _inner.Enqueue(item);
        Last = item;
    }
    //...
}

Context

StackExchange Code Review Q#65102, answer score: 5

Revisions (0)

No revisions yet.