2
\$\begingroup\$

I wrote my implementation to Queue Linked List based. And I need a review for it to improve it and improve my coding skill. I also will put this implementation on my GitHub account.

//======================================================// Author      : Omar_Hafez// Created     : 30 July 2022 (Saturday)  8:02:31 AM//======================================================#include <iostream>#include <memory>template <class T>class Queue {        private:        struct Node {            T value;            std::shared_ptr<Node> next;            Node (T data, std::shared_ptr<Node> ptr) : value(data), next(ptr) {}        };                int size_value = 0;        std::shared_ptr<Node> back;        std::shared_ptr<Node> front;    public:        enum QueueOpStatus { FailedQueueEmpty = -1, FailedQueueFull = -2, OK = 1 };        //  this constructor is to keep the consistency with the array based implementation        Queue(int MAX_SIZE = 1000000) {}        QueueOpStatus push(T const& t) {            if(full()) return FailedQueueFull;            if(front) {                back->next = std::make_shared<Node>(t, nullptr);                back = back->next;            } else {                front = std::make_shared<Node>(t, nullptr);                back = front;            }            size_value++;            return OK;        }        QueueOpStatus pop() {            if (empty()) return FailedQueueEmpty;            front = front->next;            size_value--;            return OK;        }        bool empty() const { return size_value == 0; }        bool full() const { return 0; }        int size() const { return size_value; }        T top() const {             if(empty()) {                throw "Queue is empty";            }            return front->value;         }        void clear() {            while(!empty()) {                pop();            }        }};
askedJul 30, 2022 at 6:24
Omar_Hafez's user avatar
\$\endgroup\$

1 Answer1

2
\$\begingroup\$
  • push could be streamlined:back will always point to the new node no matter what:

      auto new_node = make_shared<Node>(t, nullptr);  if (front) {      back->next = new_node;  } else {      front = new_node;  }  back = new_node;
  • The emptiness of the queue is tested differently inpush (which testsif (front)) versuspop/top (which testsempty() akasize_value == 0). Technically nothing is wrong, but it gives an uneasy feeling. Better use an uniform test.

  • top throwing an exceptioncould be an overkill. Consider returning astd::pair<bool, T>, orstd::optional<T>.

  • size_value shall besize_t.

  • To repeat another review,enqueue/dequeue are far better thanpush/pop.

janos's user avatar
janos
113k15 gold badges154 silver badges396 bronze badges
answeredJul 31, 2022 at 0:47
vnp's user avatar
\$\endgroup\$
0

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.