1
\$\begingroup\$

I have written code for the basic implementation of a Deque in C++ using Array(pointer). Please review my code and suggest ways to make it simpler, compact, and efficient. Also would appreciate feedback on any aspect of my code.

#include <iostream>
#include <cstdlib>

class queue
{
private:
    int size = 0, front = -1, rear = -1;
    float *arr;

public:
    queue(int inputSize)
    {
        size = inputSize;
        arr = (float *)malloc(size * sizeof(float));
    }
    int isEmpty(queue *q)
    {
        if (q->front == q->rear)
        {
            return 1;
        }
        return 0;
    }
    int isFull(queue *q)
    {
        if (q->rear == q->size - 1)
        {
            return 1;
        }
        return 0;
    }
    void enqueueR(queue *q, int data)
    {
        if (isFull(q))
        {
            std::cout << data << " cannot be entered as Queue is Full\n";
        }
        else
        {
            q->rear++;
            q->arr[q->rear] = data;
        }
    }
    void enqueueF(queue *q, int data)
    {
        if (isFull(q))
        {
            std::cout << data << " cannot be entered as Queue is Full\n";
        }
        else
        {
            for (int i = front + 1; i < size; i++)
            {
                q->arr[i + 1] = q->arr[i];
            }
            q->arr[q->front + 1] = data;
            q->rear++;
        }
    }
    void dequeueF(queue *q)
    {
        if (isEmpty(q))
        {
            std::cout << "Queue is Empty\n";
        }
        else
        {
            q->front++;
            std::cout << "The removed value is: " << q->arr[q->front] << "\n";
        }
    }
    void dequeueR(queue *q)
    {
        if (isEmpty(q))
        {
            std::cout << "Queue is Empty\n";
        }
        else
        {
            std::cout << "The removed value is: " << q->arr[q->rear] << "\n";
            q->rear--;
        }
    }
    void queueTraversal(queue *q)
    {
        std::cout << "The queue is as follows:\n";
        for (int i = front + 1; i <= rear; i++)
        {
            std::cout << q->arr[i] << " ";
        }
        std::cout << std::endl;
    }
};
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Advice 1

queue stores a queue of float values. Why not to rename to float_queue?

Advice 2

arr = (float *)malloc(size * sizeof(float));

You can write:

arr = new float[size];

Advice 3

Your class misses the destructor, which should call delete[] arr; on the queue. Thus, you leak RAM.

Advice 4

int size = 0, front = -1, rear = -1;

int supports some negative values, but they are meaningless in this context; change to size_t, which is concieved as the type for counting/indexing stuff.

Advice 5

int isEmpty(queue *q)
{
    if (q->front == q->rear)
    {
        return 1;
    }
    return 0;
}

Why do you ask for queue* q argument when you can simply refer to the members of the class? Same observation applies to other relevant methods.

Advice 6

if (isFull(q))
{
    std::cout << data << " cannot be entered as Queue is Full\n";
}

This is a no-no in the professional data structure development; throw an exception instead.

Advice 7

queueTraversal(queue *q)

This is asking for iterators.

Suggestion 1

I suggest you consult a professional C++-developer (obviously not me), whether it makes sense to implement:

  1. copy constructor
  2. copy assignment
  3. move constructor
  4. move assignment

All in all

I had this implementation in mind:

#include <algorithm>
#include <stdexcept>

using std::copy;
using std::logic_error;

class float_queue
{
private:
    size_t size;
    size_t max_capacity;
    size_t front_index;
    float* storage_array;

public:
    float_queue(size_t _max_capacity) : size(0),
                                        max_capacity(_max_capacity), 
                                        front_index(0), 
                                        storage_array(new float[_max_capacity]) {}

    float_queue(const float_queue& other) : 
        size(other.size),
        max_capacity(other.max_capacity),
        front_index(other.front_index) {

        storage_array = new float[other.max_capacity];
        copy(other.storage_array,
             other.storage_array + other.max_capacity, 
             storage_array);
    }

    float_queue(float_queue&& other) :
        size(other.size),
        max_capacity(other.max_capacity),
        front_index(other.front_index),
        storage_array(other.storage_array) {

        other.size = 0;
        other.max_capacity = 0;
        other.front_index = 0;
        other.storage_array = nullptr;
    }

    float_queue& operator=(const float_queue& other) {
        if (this != &other) {
            float* new_array = new float[other.max_capacity];
            copy(other.storage_array, 
                 other.storage_array + other.max_capacity, 
                 new_array);

            delete[] storage_array;
            storage_array = new_array;
            max_capacity = other.max_capacity;
            size = other.size;
            front_index = other.front_index;
        }

        return *this;
    }

    float_queue& operator=(float_queue&& other) {
        if (&other != this) {
            size = other.size;
            front_index = other.front_index;
            max_capacity = other.max_capacity;
            storage_array = other.storage_array;

            other.size = 0;
            other.front_index = 0;
            other.max_capacity = 0;
            other.storage_array = nullptr;
        }

        return *this;
    }

    ~float_queue() {
        size = 0;
        max_capacity = 0;
        front_index = 0;
        delete[] storage_array;
    }

    bool isEmpty()
    {
        return size == 0;
    }

    int isFull()
    {
        return size == max_capacity;
    }

    void prepend(float f) {
        if (isFull()) {
            throw logic_error{"Prepending while full."};
        }

        size_t new_front_index;

        if (front_index == 0) {
            new_front_index = max_capacity - 1;
        }
        else {
            new_front_index = front_index - 1;
        }

        size++;
        storage_array[new_front_index] = f;
        front_index = new_front_index;
    }
    
    void append(float f) {
        if (isFull()) {
            throw logic_error{"Appending while full."};
        }

        size_t append_index = (front_index + size) % max_capacity;
        size++;
        storage_array[append_index] = f;
    }

    float removeHead() {
        if (isEmpty()) {
            throw logic_error{"Removing a head from empty queue."};
        }

        float rv = storage_array[front_index];
        front_index = (front_index + 1) % max_capacity;
        size--;
        return rv;
    }

    float removeTail() {
        if (isEmpty()) {
            throw logic_error{"Removing a tail from empty queue."};
        }

        float rv = storage_array[(front_index + size - 1) % max_capacity];
        size--;
        return rv;
    }
};

Note that the above code snippet is not tested for bugs/design errors; it's only for my semi-professional reference. Good luck!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.