Question on using enum in class, C++

[Nathan_6]

Hi everyone.

I am trying to develop a class for a RISK boardgame, and I used UNIT_COLOR and UNIT_TYPE enums to represent the different types of pieces. These enumerations are defined as public in my class but are causing complete havoc in my code because with the C++11 "class enum," I need to start accessing my enumerations through the enum class name. However, if the enumeration is in the class itself, then I will need to access a data member from enumeration in the following way: ClassName::EnumClassName::theEnum.

This is causing havoc on my code as my compiler does not recognize that I have the same function definitions both in my header file and in the .cpp file implementation.

Let me show you what I mean:

//  Class definition of a UNIT

#ifndef Unit_hpp
#define Unit_hpp

#include <map>
#include <iostream>
//forward declaration of class to prevent header file import statement mixup
class Territory;

///////////////////////////////////////////////////////////////////////////////

//represents each type of unit that can be created and the number of "single units" in each troop, correspondingly"



//creating forward declarations of enumerations
enum class UNIT_COLOR; enum class UNIT_TYPE; enum class UNIT_COLOR;
///////////////////////////////////////////////////////////////////////////////

class Unit
{
public:
    Unit(Territory*,UNIT_TYPE&,UNIT_COLOR&);
    ~Unit();                 //destructor

    void move(Territory*);   //moves the UNIT to a new Territory

    void setAlive(const bool&);    //makes the unit alive or dead, respectively
    bool isAlive() const;          //returns true if the unit is alive

    const UNIT_COLOR getColor() const;   //returns the UNIT's color
    void setColor(UNIT_COLOR&);

    const UNIT_TYPE getUnitType() const;
    void setUnitType(UNIT_TYPE&);

    int getUnitStrength() const;

    ///////////////////////////////////////////////////////////////////////////////////

    //used for displaying a Unit's state
    friend std::ostream& operator << (std::ostream&,const Unit&);

    //these represent the possible colors that a RISK unit may have
    enum class UNIT_COLOR {YELLOW, BLUE, RED, GREEN, GREY};

    //represents the type of Risk unit
    enum class UNIT_TYPE { INFANTRY, CAVALRY, CANNON };

    static const std::map<UNIT_TYPE,int> UNIT_TO_UNIT_STRENGTH;



private:
    Territory* currentTerritoryPtr;
    int numTroops;
    UNIT_TYPE typeUnit;
    bool living;                   //represents if the unit is "living". i.e. if it can be manipulated

    UNIT_COLOR color;

    static const int MOBILITY = 1; //number of territories that a unit can move through on a single turn

};

//initializing the static map
const std::map<Unit::UNIT_TYPE,int> Unit::UNIT_TO_UNIT_STRENGTH =
{
    {Unit::UNIT_TYPE::INFANTRY,1}, {Unit::UNIT_TYPE::CAVALRY,3}, {Unit::UNIT_TYPE::CANNON,5}
};

#endif /* Unit_hpp */

and then my .cpp implementation file is:

//  Defines the member functions of the Unit class
#include "Unit.hpp"


//used for displaying a Unit's state
std::ostream& operator << (std::ostream& os,const Unit& theUnit)
{
    switch (theUnit.color)
    {
        case Unit::UNIT_COLOR::YELLOW:
            os << "Yellow";
            break;
        case Unit::UNIT_COLOR::BLUE:
            os << "Blue";
            break;
        case Unit::UNIT_COLOR::RED:
            os << "Red";
            break;
        case Unit::UNIT_COLOR::GREEN:
            os << "Green";
            break;
        case Unit::UNIT_COLOR::GREY:
            os << "Grey";
            break;
        default:
            os << "Unknown-colored";
            break;
    }

    os << " ";

    switch(theUnit.typeUnit)
    {
        case Unit::UNIT_TYPE::INFANTRY:
            os << "infantry unit";
            break;
        case Unit::UNIT_TYPE::CAVALRY:
            os << "cavalry unit";
            break;
        case Unit::UNIT_TYPE::CANNON:
            os << "cannon unit";
            break;
        default:
            os << "unit";
            break;
    }

    os << " in " << (theUnit.currentTerritoryPtr)->getName();

    if (theUnit.living)
        //the unit is alive
        os << " that is alive." << std::endl;
    else
        //the unit is not alive
        os << " that is dead." << std::endl;


    return os;
}
/*
Unit::Unit(Territory* tPtr,const UNIT_TYPE& type,const UNIT_COLOR& c): currentTerritoryPtr(tPtr),typeUnit(type), color(c)
{
    living = true;  //by default, the unit is alive at the time of creation

    numTroops = UNIT_TO_UNIT_STRENGTH.find(typeUnit)->second;
}*/
Unit::Unit(Territory* tPtr,UNIT_TYPE& type,UNIT_COLOR& c): currentTerritoryPtr(tPtr),typeUnit(type),color(c)
{
    living = true;  //by default, the unit is alive at the time of creation
    numTroops = UNIT_TO_UNIT_STRENGTH.find(typeUnit)->second;
}

Unit::~Unit()
{
    currentTerritoryPtr = nullptr;  //trying to prevent memory leak
}

void Unit::move(Territory* destinationPtr)
{
    //have the current territory delete the unit's reference to the unit
    currentTerritoryPtr->removeUnit(this);

    //represents the unit moving from a current territory
    //to the destination. assuming that the destination
    //reference by the destinationPtr has been vetted.

    currentTerritoryPtr = destinationPtr;

    //then have the new territory accept the unit
    currentTerritoryPtr->takeInUnit(this);
}

void Unit::setAlive(const bool& newState)
{
    living = newState;
}

bool Unit::isAlive() const
{
    return living;
}

const Unit::UNIT_COLOR Unit::getColor() const
{
    return color;
}

void Unit::setColor(UNIT_COLOR& newC)
{
    color = newC;
}

const UNIT_TYPE Unit::getUnitType() const
{
    return typeUnit;
}

void Unit::setUnitType(UNIT_TYPE& newType)
{
    typeUnit = newType;
    numTroops = UNIT_TO_UNIT_STRENGTH.find(typeUnit)->second;
}

int Unit::getUnitStrength() const
{
    return numTroops;
}

And then I get the following errors for my .cpp file implementation (keep in mind that this is on xCode which guides me through formatting my code as well, and xCode was happy with my header file):

• For the constructor "Out of line definition of 'Unit' does not match any declaration in 'Unit'
  *For the member function getColor(), it returns "Return type of out-of-line definition of Unit::getColor differs from that in the declaration.

  OK. Fine. I just went to the header file and changed my return type for getColor() from UNIT_COLOR to Unit::UNIT_COLOR to match what was in the .cpp file. However, xCode prompted me to delete the "Unit::"" and to just keep the return type "UNIT_COLOR," so I do not know how to fix this issue.
  

• For setColor, "Out of line definition of setColor does not match any declaration in 'Unit'

A few other functions also have an error similar to that of setColor including setUnitType and getUnitType(). something in these enums is causing me havoc.

I have been pouring over this for many hours. I was wondering if any of you programmers would have an insight or see something instantly that I am not seeing and then care to weigh in.

Thank you very much for your help in advance.

[AssertNull]

I created a project in NetBeans and compiledwith -std=c++11. You don't provide a main or the Territory files, so I made those and got it to compile. Obviously my main and Territory files were bare-bone, so potentially there could be problems when you compile/link the real ones. Here's what I did. Give it a try.

First, I got rid of the forward declarations of the UNIT_TYPE and UNIT_COLOR enums (delete line 18 of Unit.hpp). Second, I stuck those enums at the TOP of my unit class definition (move lines 45-49 to right before line 24). Third, I added #include "Territory.hpp" at the top of the Unit.hpp file. Fourth, I moved lines 67 - 71 of Unit.hpp (initialization of static map) from the bottom of Unit.cpp to the top of Unit.cpp (after the #includes). You want the declaration in the header file, but the initialization in the cpp file. Finally, I Added Unit:: to any functions in Unit.cpp that returned an enumerated type. That appears to only need to happen on line 113. You did it right on line 103. Use that as a model, so line 113 becomes:

const Unit::UNIT_TYPE Unit::getUnitType() const

Unless I missed something, that's all that needs to be done. Of course, again, I don't have your other files, like the Territory.hpp and .cpp files, so more may need to be done.

[Nathan_6]

Third, I added #include "Territory.hpp" at the top of the Unit.hpp file.

Hi, and thank you for being willing to provide some input. I remember that in my C++ how to program book they said that including header files in each other could result in some compilation issues if there was circular referencing of header files. That is why I included the forward class declaration of Territory in the code that I provided. Do you think that I should do the #include "Territory.hpp" for sure?

I got my code to compile with your help without including the Territory.hpp file using a forward declaration of class Territory instead. So the question is whatever is a safer/better practice, including, or not including a header file with a forward class declaration. What do you think is better?

Thank you again

[AssertNull]

Forward declarations are fine. I felt that the forward declaration of the enumerated types UNIT_COLOR and UNIT_TYPE were better as actual members of the Unit class. Regarding including the Territory.hpp file in Unit.hpp file versus forward declaring it, I recall experimenting around a little bit till something worked, which doesn't mean it's the ONLY thing that would work. The #include's can be tricky, but if you have the right #ifndef guards in there and order things right, it can work. Short answer, if it compiled and linked correctly, go with it. I imagine forward declarations give the preprocessor less work to do and the compiler/linker less to parse, so everything else being equal, if the forward declaration works and you don't have to #include a file, I guess I would vote in general for using the forward declaration if it allows you to not have to #include the file. But that's a rule of thumb; every project is different.

This one was small enough and easy enough and I had to create bare-bones Territory and main files and just moved stuff around till it worked and seemed like it made sense, then posted what I did. Again, there can be more than one solution. The key point is to make a decision on placement and try to be as consistent as possible.

If you have something that works, go with it.