Queue/stack/node class Implementation

Question

Below there are 3 short header files for queue, stack, and node implementation. I do understand that the STL has this capability(I am just doing it for practice). Please criticize the coding style, and things that you might thing i can/should do better. thank you. Note: since they are pretty short and Node class is used in the other two i posted all in here.

Node.h:
----------------------------------------------------
// Node.h
// 1-way linked node for use in simple integer Queue

#ifndef NODE_H
#define NODE_H


class Node
{
public:
    Node();
    Node(int);

    int data;
    Node *next;
};

#endif

//-----------------------------//

 Node.cpp:
-----------------------------------------
#include "Node.h"

Node::Node()
{
    data = -1;
    next = nullptr;
}

Node::Node(int x)
{
    data = x;
    next = nullptr;
}

fmStack.h:
------------------------------------------------
#ifndef _FMSTACK_H
#define _FMSTACK_H

#include <iostream>
#include "Node.h"

namespace fm
{

    class fmStack
    {
        Node *top;

    public:

        fmStack();
        ~fmStack();

        void push(int);
        void pop();
        void dumpStack();

    };


}
#endif /* _FMSTACK_H */

//-------------------------------//

fmStack.cpp:
-----------------------------------------------
#include "fmStack.h"
using namespace fm;
//-------Private Methods----------

//-------Public Methods-----------
fmStack::fmStack()
{
    top = nullptr;
}

fmStack::~fmStack()
{
    top = nullptr;
}

void fmStack::push(int x)
{
    Node *node = new Node;

    node->data = x;
    node->next = nullptr;

    if (top != nullptr)
        node->next = top;

    top = node;
}

void fmStack::pop()
{
    Node *node;

    if (top == nullptr)
        std::cout << "Stack is empty" << std::endl;

    node = top;
    top = top->next;
    std::cout << "Poped from stack" << std::endl;
        delete node;


}

void fmStack::dumpStack()
{
    Node *node = top;
    if (top == nullptr)
        std::cout << "Stack is empty" << std::endl;

    while (node != nullptr)
    {
        std::cout << "data in stack at current position is" << node->data << std::endl;
        node = node->next;
    }

}

fmQueue.h:

#ifndef _FMQUEUE_H
#define _FMQUEUE_H


#include <iostream>
#include "Node.h"

namespace fm
{
    class fmQueue
    {
        Node *_head, *_tail;
        void clearbuf();


    public:

        fmQueue();

        ~fmQueue();

        void deQueue(); // uses front to access data, or remove data
        void enQueue(int); // uses back to sort data, or add data
        void dumQueue();

        //int peek(); // get a copy of the front data without removing it

        bool isEmpty();

    };

}

#endif /* _FMQUEUE_H */

//---------------------------------//

fmQueue.cpp:

#include "fmQueue.h"

using namespace fm;

//---------Private Methods--------
void fmQueue::clearbuf()
{
    _head = _tail = nullptr;
}

//--------Public Methods----------

fmQueue::fmQueue()
{
    clearbuf();
}

fmQueue::~fmQueue()
{
    clearbuf();
}

bool fmQueue::isEmpty()
{
    if (_head == _tail && _head == nullptr)
        return false;
    else
        return true;
}


void fmQueue::enQueue(int data1)
{
    Node *tempNode = new Node;

    tempNode->next = nullptr;
    tempNode->data = data1;

    if (_head == nullptr)
    {
        _head = tempNode;
        _tail = tempNode;
    }


    else
    {
        _tail->next = tempNode;
    }

    _tail = tempNode;
}

void fmQueue::deQueue()
{
    Node *tempNode = new Node;

    if (_head == nullptr)
        std::printf("NOOOOP, THE QUEUE IS EMPTY");

    else
    {
        tempNode = _head;

        _head = _head->next;
        std::cout << "the data dequeued is: " << tempNode->data; //add a print statment to see which node was deleted

        delete tempNode;


    }

}

void fmQueue::dumQueue()
{
    Node *tempNode = new Node;

    if (tempNode)
        while (tempNode->next != nullptr)
        {
            std::cout << "Queue :" << tempNode->data;

            tempNode = tempNode->next;
        }
    else
        std::cout << "Nothing to show";

}

miscco · Accepted Answer · 2017-06-24 06:31:16Z

You should realize, that your stack implementation leaks memory, if a non empty stack goes out of scope. In that case you have to recursively call delete on your nodes.

To remedy this you should look into std::unique_ptr that handles the memory management for you.
Why does your stack doesnt have a function to retrieve the top node?
Is there any reason you defined int rather than making a template library?
dumpStack is a bad name, as dumping semantically involves throwing away. Maybe printStack?

Use correct constructors and initializer lists

class Node
{
    public:
    Node() {};
    Node(const int value)
       : data(value)   
    {}
    Node(const int value, Node* nextNode)
       : data(value)
       , next(nextNode)   
    {}

    int data = -1;
    Node *next = nullptr;
};

You can even simplify this itno a single one

Node(const int value = -1, Node* nextNode = nullptr)
       : data(value)
       , next(nextNode)   
    {}

    int data;
    Node *next;
};

Then you push gets way easier

void fmStack::push(int x)
{
    Node *node = new Node(x, top);    
    top = node;
}

You pop is seriously flawed. If the stack is empty, you still access top. In that case better throw a std::exception or at least do an early return

1- I have to make a deconstruct method for my Node class. (I think) 2-I Believe in the pop method, node will be pointing to the top. — BlooB
– BlooB, Commented Jun 25, 2017 at 0:14
4-I will change that 5-I have never looked into creating constructor/initializer that way. I will do so 6-I believe i have to wrap the staments in a else statment — BlooB
– BlooB, Commented Jun 25, 2017 at 0:26

Stack Exchange Network

Queue/stack/node class Implementation

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Queue/stack/node class Implementation

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions