Basic Queue in C++ Using Array

Question

I wrote a queue class in C++ using arrays of fixed width. Could anyone review my code ? I would appreciate any comment and recommendations. It works like a circular queue, so I handled back and front pointers in that manner.

#include <stdlib.h>
#include <iostream>
template <class T>
class Queue {
 public:
 Queue(void);
 Queue(Queue<T>& copyQueue);
 bool empty(void) const;
 std::size_t size(void) const;
 void clear(void);
 T front(void) const;
 void pop(void);
 void push(T& item);
 void print(std::ostream& os);
 private:
 static const std::size_t MAX_SIZE = 50;
 T list[MAX_SIZE];
 T* frontPtr;
 T* backPtr;
 std::size_t sizeQ;
};
template <class T>
Queue<T>::Queue(void) {
 frontPtr = nullptr;
 backPtr = nullptr;
 sizeQ = 0;
}
template <class T>
Queue<T>::Queue(Queue<T>& copyQueue) {
 frontPtr = nullptr;
 backPtr = nullptr;
 sizeQ = 0;
 if(copyQueue.backPtr >= copyQueue.frontPtr){
 for(T* i = copyQueue.frontPtr; i <= copyQueue.backPtr; i++){
 push(*i);
 }
 }
 else {
 for(T* i = copyQueue.frontPtr; i <= (copyQueue.list + MAX_SIZE -1); i++){
 push(*i);
 }
 for(T* i = copyQueue.list; i <= copyQueue.backPtr; i++) {
 push(*i);
 }
 }
}
template <class T>
bool Queue<T>::empty(void) const {
 return (sizeQ == 0);
}
template <class T>
std::size_t Queue<T>::size(void) const {
 return sizeQ;
}
template <class T>
void Queue<T>::clear(void) {
 frontPtr = nullptr;
 backPtr = nullptr;
 sizeQ = 0;
}
template <class T>
T Queue<T>::front(void) const {
 if(frontPtr == nullptr) {
 std::cerr << "Queue is empty. No front value" << '\n';
 }
 else {
 return *frontPtr;
 }
}
template <class T>
void Queue<T>::pop(void) {
 if(sizeQ == 0) {
 std::cerr << "Queue is empty. Can't pop." << '\n';
 }
 else{
 frontPtr = list + ((frontPtr - list + 1) % MAX_SIZE);
 sizeQ -= 1;
 }
}
template <class T>
void Queue<T>::push(T& item) {
 if(sizeQ == MAX_SIZE) {
 std::cerr << "Queue is full. Can't push" << '\n';
 }
 else{
 if(sizeQ == 0) {
 frontPtr = backPtr = list;
 }
 else {
 backPtr = list + ((backPtr - list + 1) % MAX_SIZE);
 }
 *backPtr = item;
 sizeQ += 1;
 }
}
template <class T>
void Queue<T>::print(std::ostream& os) {
 if(backPtr >=frontPtr){
 for(T* i = frontPtr; i <= backPtr; i++){
 os << *i << '\n';
 }
 }
 else {
 for(T* i = frontPtr; i <= (list + MAX_SIZE -1); i++){
 os << *i << '\n';
 }
 for(T* i = list; i <= backPtr; i++) {
 os << *i << '\n';
 }
 }
}

Edit: Not using googletest, but a simple main function to test the Queue.h can be found below:

#include "Queue.h"
int main() {
 Queue<int> newQueue;
 for(int i=1; i<110; i = i+2)
 newQueue.push(i);
 newQueue.pop();
 newQueue.pop();
 newQueue.pop();
 newQueue.pop();
 int i = 999;
 newQueue.push(++i);
 newQueue.push(++i);
 newQueue.push(++i);
 newQueue.push(++i);
 newQueue.print(std::cout);
 Queue<int> copiedQueue(newQueue);
 std::cout << copiedQueue.size() << '\n';
 copiedQueue.print(std::cout);
 for(int i=1; i<11; i = i+2)
 copiedQueue.pop();
 newQueue.print(std::cout);
 std::cout << copiedQueue.empty();
 std::cout << newQueue.empty();
 copiedQueue.print(std::cout);
 return 0;
}

• pacmaninbw: You've obviously put a lot of time into this, could you please add some code that uses this class or tests it?
• Erdem Tuna: @pacmaninbw , I added a main function to test the code.

Answer 1

Some general improvements:

1. Use <cstdlib>, not <stdlib.h>. The latter is a deprecated header that is kept for C compatibility. It should not be used in new C++ code.
2. Do not use void in an empty parameter list. It is counterintuitive and is not necessary in C++ at all. It is only used in C prototypes.
3. The copy constructor should take by const reference because it does not modify the argument. Same for push.
4. You are using assignment in constructors when you ought to use member initializer clauses. This is bad practice.
5. front should return by const reference, not by value. Returning by value makes an unnecessary copy.
6. MAX_SIZE is not a macro and should not be in ALL_CAPS. And it should be constexpr. Or better, a template parameter.
7. It is advised in C++ to use std::array instead of raw arrays.

There are still many things to improve, but this should be enough to get you started.

• Erdem Tuna: thank you for having time to analyze and elaborate the class. Regarding your 8th suggestion, shall I remove the sizeQvariable? Then, I should track the fullness of the queue by backPtr - frontPtr? This would create problem as it is a circular queue. Could you please inform me a bit more on this?
• L. F.: @ErdemTuna Oops, I didn't realize that this is a circular queue. Sorry for that. Still, the sizeQ variable contains more information than it should and you have to keep it synchronized. You may replace it with a bool variable indicating whether the queue is empty, I guess. I'll delete that bullet.

Answer 2

1) swap from using an array to either vector or std::array, N> elems; current when using std::array the types that you may hold in you queue are limited to default constructable

2) copy constructor should be passed by const ref

3) Perhapse think about moving the max size parameter as a template perameter template class Queue

4) I noticed there is not a assignment operator is this on purpose?

5) correct me if I am wrong but when implementing a circular queue and a push runs out of space shouldn't you pop from the front and push to back

• Erdem Tuna: Thank you for the evaluation. Regarding 3, why and how should MAX_SIZE be a template class? Regarding 4, I just didn't implement it, I should write that part. Regarding 5, I wasn't aware of that fact, but I guess you are right.
• swaggg_pickle: @ErdemTuna 3 - why? Because it would allow for a more flexible use case of the class now the max size can still be determined at compile time but different instances can have different max sizes if you wish. How? c++ template <class T, std::size_t MAX_SIZE = 50> class Queue {