Queue with std::vector

Question

How can I improve this code?

template<class T>
class Queue {
public:
    Queue(const std::initializer_list<T>& i) :elem{i} {}
    int size() const {
        return elem.size();
    }
    bool empty() const {
        return elem.empty();
    }
    void enqueue(T&&);
    void dequeue();
    T peek();
    T& operator[](std::size_t);
    typename std::vector <T>::iterator begin() {
        return elem.begin();
    }
    typename std::vector <T>::iterator end() {
        return elem.end();
    }

private:
    std::vector<T> elem;
};

template<class T>
void Queue<T>::enqueue(T&& t) {
    elem.push_back(t);
}

template<class T>
void Queue<T>::dequeue() {
    if (empty()) {
        throw std::out_of_range("underflow");
    }
    elem.erase(elem.begin());
}

template<class T>
T Queue<T>::peek() {
    if (empty()) {
        throw std::out_of_range("underflow");
    }
    return elem.front();
}

template<class T>
T& Queue<T>::operator[](std::size_t i) {
    if (empty() || i < 0 || i >= size()) {
        throw std::out_of_range("underflow");
    }
    return elem[i];
}

Answer 1

I assume that you are familiar with std::queue and std::deque and wrote this class as a reinventing-the-wheel exercise.

There are a few mainly aesthetic points that you could improve:

• int size() is better if returning a std::size_t, which is the underlying type returned by vector::size().

• Keep parameter names in function prototypes, it helps self-documenting the code.

• T peek() doesn't change any internal state, so it should be const too.

• operator[] should be provided with two overloads, const and non-const:

    T& operator[](std::size_t index);
    const T& operator[](std::size_t index) const;
  

Otherwise you won't be able to access data in a const Queue<T>. The same thing applies to begin()/end(). You will have to provide both const and non-const versions. This is purely boilerplate code, but C++ doesn't offer a better solution.

• This excessively long name: typename std::vector <T>::iterator should be replaced by a typedef or using alias:

    using iterator = std::vector<T>::iterator;
  
    // Then you simply use `iterator` now:
    iterator begin() { ... }
  

• Shouldn't you provide a constructor that takes no arguments? It is very likely that you will want to declare an empty queue at some point. A default constructor should also do:

    Queue() = default;