Struct and class for queue

Question

When I build this code, it doesn't print out anything. What is wrong with this code, I used one of the codes on the internet as a reference, it used a struct to declare queue. But I just wanted to use class. Is there any problem with using the object of the class? What are some other problems in this code?

#include<bits/stdc++.h>
using namespace std;

    class Queue{
    public:
        int front, rear, size;
        unsigned capacity;
        int* array;
    };
    
    Queue* createQueue(unsigned capacity){
        Queue* queue = new Queue;
        queue->front = queue->size = 0;
        queue->capacity = capacity;
        queue->rear = queue->capacity-1;
        queue->array = new int[queue->capacity];
    }
    
    int isEmpty(Queue* queue){
        return (queue->size==0);
    }
    
    int isFull(Queue* queue){
        return (queue->size==queue->capacity);
    }
    
    void enqueue(Queue* queue, int item){
        if(isFull(queue)){
            return;
        }else{
            queue->rear = (queue->rear+1)%queue->capacity;
            queue->size = queue->size+1;
            queue->array[queue->rear] = item;
            cout << item << " enqueued to queue\n";
        }
    }
    
    int dequeue(Queue* queue){
        if(isEmpty(queue)){
            return NULL;
        }else{
            int temp = queue->array[queue->front];
            queue->front = (queue->front+1)%queue->capacity;
            queue->size = queue->size-1;
            return temp;
        }
    }
    
    int front(Queue* queue){
        if(isEmpty(queue)){
            return 0;
        }else{
            return queue->array[queue->front];
        }
    }
    
    int rear(Queue* queue){
        if(isEmpty(queue)){
            return NULL;
        }else{
            return queue->array[queue->rear];
        }
    }
    
    int main(){
        Queue queue;
        Queue* ptr = &queue;
        enqueue(ptr, 10);
    }

Were you planning on doing something with `createQueue` ? I big hint that something went off the rails is that function seems destined to... *create a queue*, yet is never used in any of this code. Besides the bug in that function (it never returns a value), right now `Queue queue;` default-constructs a `Queue`, which never initializes any member variables to determinate values. Therefore, later expressions like `return (queue->size==queue->capacity)` in `isFull` are relying on indeterminate values, and invoke *undefined behavior*. This literally looks like a bad C queue port. — WhozCraig, Apr 05 '21 at 04:07
Thank you for your comment. I missed the createQueue to function to initialize the members. Thank you so much. — MinKwon Kim, Apr 05 '21 at 04:15
Is there some requirement of being compatible with a C lib or is there any other reason for not making all the functions except for `main` and `createQueue` member functions and moving the logic of `createQueue` to an constructor? — fabian, Apr 05 '21 at 06:31
[`#include`<-- don't do that. It's a compiler header and not part of the standard.](https://stackoverflow.com/a/31816096) — JHBonarius, Apr 05 '21 at 07:02

JHBonarius · Answer 1 · 2021-04-05T07:43:45.567

Many things:

you expose all of your Queue class member variables, defying the whole idea encapsulation. (don't expose your member to others)
preferably don't use an unsigned type for a size or capacity. unsigned arithmetic doesn't always behave the way you expect it to.
You should encapsulate the functions with the type. I.e. enqueue should be a member function of Queue. (At least if you want to use OO)
don't return int from a predicate. use bool.
createQueue is defined to return Queue*, however, you don't return anything. That's Undefined Behavior.
dequeue can return NULL. NULL is used for a null pointer. but the function is defined to return an int, so that's wrong. Furthermore, in C++ we use nullptr. If you conditionally want to return somehting, use std::optional
you are handling a raw pointer (array), but don't have a destructor, copy/move constructor, copy/move-assignment operators defined. So you'll have a lot of memory leaks. use the rule of five. Of better: use std::vector.

The whole thing could look something like (NEEDS MORE TESTS)

#include <vector>
#include <iostream>
#include <optional>

class Queue{
private:
    int capacity;
    std::vector<int> data{};
public:
    Queue(int capacity)
    : capacity(capacity)
    {}

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

    bool isFull(){
        return data.size()==capacity;
    }

    bool enqueue(int item){
        if (isFull()){
            return false;
        } else {
            data.push_back(item);
            std::cout << item << " enqueued to queue\n";
            return true;
        }
    }

    std::optional<int> dequeue(){
        if (isEmpty()) {
            return std::nullopt;
        } else {
            int retval = data.front();
            data.erase(data.begin());
            return retval;
        }
    }

    std::optional<int> front(){
        if (isEmpty()) {
            return std::nullopt;
        }else{
            return data.front();
        }
    }

    std::optional<int> back(){
        if (isEmpty()) {
            return std::nullopt;
        }else{
            return data.back();
        }
    }
};

int main(){
    Queue queue(5);
    queue.enqueue(10);
    std::cout << "removed from queue: " << queue.dequeue().value() << '\n';
    std::cout << "Queue contains nothing: " << (queue.back().has_value() == false?"yes":"no") << '\n';
}

Struct and class for queue

1 Answers1