how to fix multiple definition error in c++?

Question

I tried to look at other related posts but I'm sill stuck

My header files looks something like this

Node.hpp

      #include<iostream>
using namespace std;
#ifndef NODE_HPP
    #define NODE_HPP



        struct Node
        {
            int value;
             Node *start;
             Node *end;
        }
         *start, *end;
         int count= 0;


        #endif

and

Queue.hpp

  #include<iostream>
using namespace std;
#ifndef QUEUE_HPP
#define QUEUE_HPP
#include "Node.hpp"

class Queue{
    public:
        Node *nNode(int value);
        void add(int value);
        void remove();
        void display();
        void firstItem();
        Queue()
        {   
            start = NULL;
            end = NULL;
        }   
    };
    #endif

My implementation for queue looks like

#include<iostream>
using namespace std;

#include "Queue.hpp"
#include<cstdio>
#include<cstdlib>

And main looks like

  #include<iostream>
    using namespace std;
    #include "Queue.hpp"
    #include<cstdio>
    #include<cstdlib>

I'm getting the following errors

 /tmp/ccPGEDzG.o:(.bss+0x0): multiple definition of `start'
    /tmp/ccJSCU8M.o:(.bss+0x0): first defined here
    /tmp/ccPGEDzG.o:(.bss+0x8): multiple definition of `end'
    /tmp/ccJSCU8M.o:(.bss+0x8): first defined here
    /tmp/ccPGEDzG.o:(.bss+0x10): multiple definition of `count'
    /tmp/ccJSCU8M.o:(.bss+0x10): first defined here

What am I doing wrong here?

Answer 1

Others already have explained the cause of the error: you are defining the same global variables in multiple translation units.

A possible fix is to define them in just one point and declare them as extern in the header file.

However, in my opinion the real question is: do you really need those global variables? If they are meant to be part of the state of a queue object, we should make them instance variables.

class Queue{
    public:
        Node *nNode(int value);
        void add(int value);
        void remove();
        void display();
        void firstItem();
        Queue()
        {   
            start = NULL;
            end = NULL;
        }   
        Node *start, *end; // <----
    };

In this way we can use multiple queue objects at runtime, and each of them will manage its own data. By comparison, instantiating many objects of the original class is useless, since they will all operate on the same queue.

TL;DR: encapsulate your state, avoid globals.