Compiler says every constructor and function is being redefined and won't compile. Am I using the #IFNDEF wrong? - C++

Question

I am trying to implement my own OrderedList data structure using class templates. Each constructor and function in my CPP file has an error on the opening '{'. The error reads "redefinition of FUNCTION_NAME FUNTION_NAME previously declared here".

I've had a few colleagues look at it, but to no avail. Here are my files and the error:

CPP FILE "MyOrderedList.cpp"

#include "MyOrderedList.h"

template <class E>
MyOrderedList<E>::MyOrderedList() {/*IMPLEMENTATION*/}

template <class E>
MyOrderedList<E>::MyOrderedList(const MyOrderedList<E>& orig) { /*IMPLEMENTATION*/}

template <class E>
void MyOrderedList<E>::operator =(const MyOrderedList<E>& orig){/*IMPLEMENTATION*/}

template <class E>
MyOrderedList<E>::~MyOrderedList() {/*IMPLEMENTATION*/}

template <class E>
void MyOrderedList<E>::insert(E data){/*IMPLEMENTATION*/}

template <class E>
E MyOrderedList<E>::get(int pos) const{/*IMPLEMENTATION*/}

template <class E>
Node<E>* MyOrderedList<E>::getHead() const{/*IMPLEMENTATION*/}

template <class E>
MyOrderedList<E> MyOrderedList<E>::kLargest(int k) const{/*IMPLEMENTATION*/}

template <class E>
MyOrderedList<E> MyOrderedList<E>::operator +(const MyOrderedList<E>& list {/*IMPLEMENTATION*/}

H FILE "MyOrderedList.h"

#ifndef MYORDEREDLIST_H
#define MYORDEREDLIST_H

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

template <class E>
class MyOrderedList;
template <class E>
std::ostream& operator <<(std::ostream& out, const MyOrderedList<E>& list);

template <class E>
class MyOrderedList {
public:
    MyOrderedList();
    MyOrderedList(const MyOrderedList<E>& orig);
    void operator =(const MyOrderedList<E>& orig);
    virtual ~MyOrderedList();

    bool remove(E data);
    MyOrderedList<E> kLargest(int k) const;
    E get(int pos) const;
    void insert(E data);
    Node<E>* getHead() const;

    MyOrderedList<E> operator +(const MyOrderedList<E>& list);

    friend std::ostream& operator <<(std::ostream& out, const MyOrderedList<E>& list){/*IMPLEMENTATION*/}

private:
    Node<E>* head;
    int size;
};

#include "MyOrderedList.cpp"
#endif  //MYORDEREDLIST_H

Node.h

#ifndef NODE_H
#define NODE_H

#include <iostream>

template <class E>
class Node {
public:
    Node(const E& init_data = NULL, Node<E>* init_link = NULL){data = init_data; link = init_link;}
    Node(const Node<E>& orig);
    virtual ~Node();

    E getData() const{return data;}
    void setData(E newData);

    Node<E>* getLink(){return link;}
    void setLink(Node<E>* nextLink) {link = nextLink;}
private:
    E data;
    Node<E>* link;
};

#endif  /* NODE_H */

Compiler Errors

MyOrderedList.cpp:12: error: redefinition of `MyOrderedList<E>::MyOrderedList()'
MyOrderedList.cpp:12: error: `MyOrderedList<E>::MyOrderedList()' previously declared here
MyOrderedList.cpp:19: error: redefinition of `MyOrderedList<E>::MyOrderedList(const MyOrderedList<E>&)'
MyOrderedList.cpp:19: error: `MyOrderedList<E>::MyOrderedList(const MyOrderedList<E>&)' previously declared here
MyOrderedList.cpp:42: error: redefinition of `void MyOrderedList<E>::operator=(const MyOrderedList<E>&)'
MyOrderedList.cpp:42: error: `void MyOrderedList<E>::operator=(const MyOrderedList<E>&)' previously declared here
MyOrderedList.cpp:77: error: redefinition of `MyOrderedList<E>::~MyOrderedList()'
MyOrderedList.cpp:77: error: `virtual MyOrderedList<E>::~MyOrderedList()' previously declared here
MyOrderedList.cpp:91: error: redefinition of `void MyOrderedList<E>::insert(E)'
MyOrderedList.cpp:91: error: `void MyOrderedList<E>::insert(E)' previously declared here
MyOrderedList.cpp:119: error: redefinition of `E MyOrderedList<E>::get(int) const'
MyOrderedList.cpp:119: error: `E MyOrderedList<E>::get(int) const' previously declared here
MyOrderedList.cpp:134: error: redefinition of `Node<E>* MyOrderedList<E>::getHead() const'
MyOrderedList.cpp:134: error: `Node<E>* MyOrderedList<E>::getHead() const' previously declared here
MyOrderedList.cpp:140: error: redefinition of `MyOrderedList<E> MyOrderedList<E>::kLargest(int) const'
MyOrderedList.cpp:140: error: `MyOrderedList<E> MyOrderedList<E>::kLargest(int) const' previously declared here
MyOrderedList.cpp:158: error: redefinition of `MyOrderedList<E> MyOrderedList<E>::operator+(const MyOrderedList<E>&)'
MyOrderedList.cpp:158: error: `MyOrderedList<E> MyOrderedList<E>::operator+(const MyOrderedList<E>&)' previously declared here

Answer 1

Your problem is that you are including the definition of the functions in the header (indirectly through the #include "MyOrderedList.cpp". Because the functions are not declared as inline the compiler is generating the symbols in all translation units that include the header and thus you get multiply defined symbols.

One solution to the problem is declaring the functions as inline in the definition, so that the linker can discard the duplicates. Keeping the definitions in a separate file is not something new, but it is not too common either, some standard library implementations do that. But consider renaming the .cpp to something else to avoid confusion (most people will think that .cpp means that it is meant to be compiled, not included).

I would recommend, on the other hand, to simplify it a bit more, remove the .cpp altogether and just have the .h with everything (still mark all function definitions as inline) or even go one step further and define the functions inside the class definition, where they are inline by default.

Answer 2

Your .cpp and .h files #include each other. Why?

Answer 3

The declarations and definitions for template classes need to stay together. There's nothing wrong with using #include on your .cpp file in order to do this, however your .cpp file will also need header guards (i.e. #ifndef MYORDEREDLIST_CPP etc). The usual approach however is simply to shove everything into the same .h file

Note - template classes/functions are the only time you should ever need to #include a .cpp file or put definitions into a header file.