patterncppMinor
Queue with std::vector
Viewed 0 times
withstdqueuevector
Problem
How can I improve this code?
template
class Queue {
public:
Queue(const std::initializer_list& 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 ::iterator begin() {
return elem.begin();
}
typename std::vector ::iterator end() {
return elem.end();
}
private:
std::vector elem;
};
template
void Queue::enqueue(T&& t) {
elem.push_back(t);
}
template
void Queue::dequeue() {
if (empty()) {
throw std::out_of_range("underflow");
}
elem.erase(elem.begin());
}
template
T Queue::peek() {
if (empty()) {
throw std::out_of_range("underflow");
}
return elem.front();
}
template
T& Queue::operator[](std::size_t i) {
if (empty() || i = size()) {
throw std::out_of_range("underflow");
}
return elem[i];
}Solution
I assume that you are familiar with
There are a few mainly aesthetic points that you could improve:
-
-
Keep parameter names in function prototypes, it helps self-documenting the code.
-
-
Otherwise you won't be able to access data in a
-
This excessively long name:
-
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:
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. 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 ::iterator should be replaced by a typedef or using alias:using iterator = std::vector::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;Code Snippets
T& operator[](std::size_t index);
const T& operator[](std::size_t index) const;using iterator = std::vector<T>::iterator;
// Then you simply use `iterator` now:
iterator begin() { ... }Queue() = default;Context
StackExchange Code Review Q#75144, answer score: 6
Revisions (0)
No revisions yet.