Unidentified identifier c++, but it's identified

Question

having this issue with Country or Continent not being identified in the following code for the Map.h file

Map.h:

#ifndef MAP_H
#define MAP_H
#include<string>
#include<vector>
#include<iostream>
#include<fstream>
//#include "Country.h"

using namespace std;
class Map{

private:
    vector<Country> countries;
    vector<Continent> continents;
    vector<vector<int> > adjacents;
    string author;
    string image;
    string wrap;
    string scroll;
    string warn;
public:
    Map(ifstream);
    Map();
    void save();
    void setAdjacent(Country&, Country&);
    void placeWithin(Continent&, Country&);
    int numOfCountries();
    int numOfContinents();
    bool verify();
    bool isAdjacent(Country*, Country&);
    bool hasAdjacent(Country*);
    bool hasCountry(Continent&);

};
#endif

Whether add in the commented out include, i still get the error.

Here are the other two files

Continent.h:

#ifndef CONTINENT_H
#define CONTINENT_H

#include "Map.h"
using namespace std;
class Continent
{
private:
    string name;
    int bonus;
    vector<Country> countries;
public:
    void addCountry(Country&);
    int getOwner();
    int getBonus();
    int getSize();
    string getName();
    bool hasCountry(Country&);


};
#endif

Country.h:

#ifndef COUNTRY_H
#define COUNTRY_H
#include "Continent.h"


using namespace std;

class Country{
    private:
        static int nextCountryNumber;
        int countryNumber;
        Map map;
        string name;
        int x, y;
        Continent continent;
        vector<Country> adjacents;
    public:
        Country(string, int, int, Continent&, Map&);
        string getName();
        int getX();
        int getY();
        Continent getContinent();
        bool isAdjacentTo(Country&);
        bool hasAdjacent();
        void addAdjacent(Country&);
        string toString();
};
#endif

I think its some sort of circular referencing but I cant find where that is the case...

Answer 1

Generally in .h files you don't want to include other header files. They're just declarations, so they don't need to know what the other classes look like. Remove the #include "blah.h" and instead write forward declarations class blah; where you need them. This is the case UNLESS you store an object blah directly in your class, in which case you need to include blah.h because the compiler needs to know how big that object is. You can avoid these includes if instead you store pointer-to-object instead of the object directly (and indeed this is a more common practice, as you don't need to copy all the data when you copy construct).

EDIT: As a side note, you generally only want to include the necessary #includes (namely iostream), and also not use using namespace std; in your headers, because another file including your header might not want to use the std namespace. Instead, do these things in your implementation file. I've made the corresponding changes below.

EDIT 2: Also, keep in mind that map is a data structure type in std, so be careful when you name things map (I'd advise using a different name).

EDIT 3: As pointed out in the comments std containers require the full declaration of the objects they contain, so whenever you have vector<blah> you must include blah.h. Also consider this: it's problematic to include an object blah within its own declaration. Since the object contains an instance of itself, that's recursive, so how much space should be allocated? You can sorta fix this by having pointer-to-blah in blah (a pointer is just an int so fixed size). For a similar reason it's problematic for the Country class to contain a vector<Country>. THAT can be solved by changing vector<Country> to vector<Country*>. So to make these technically compilable by the C++ standard and to clean up the design with respect to including other headers, I've changed the member variables referencing Country, Continent, and Map to their respective pointers, a change that you will need to mirror in the implementation files. Also I've modified Map(std::ifstream); to Map(std::ifstream &); as suggested by another comment, as I seriously doubt you intended to copy an ifstream here.

Code:

Map.h:

#ifndef MAP_H
#define MAP_H
#include<string>
#include<vector>
#include<fstream>

class Country;
class Continent;

class Map{

private:
    std::vector<Country*> countries; // consider pointers instead
    std::vector<Continent*> continents; // consider pointers instead
    std::vector<std::vector<int> > adjacents; // (just ints, so no pointers needed)
    std::string author;
    std::string image;
    std::string wrap;
    std::string scroll;
    std::string warn;
public:
    Map(std::ifstream &);
    Map();
    void save();
    void setAdjacent(Country&, Country&);
    void placeWithin(Continent&, Country&);
    int numOfCountries();
    int numOfContinents();
    bool verify();
    bool isAdjacent(Country*, Country&);
    bool hasAdjacent(Country*);
    bool hasCountry(Continent&);

};
#endif

Continent.h:

#ifndef CONTINENT_H
#define CONTINENT_H
#include<string>
#include<vector>

class Country;

class Continent
{
private:
    std::string name;
    int bonus;
    std::vector<Country*> countries;
public:
    void addCountry(Country&);
    int getOwner();
    int getBonus();
    int getSize();
    std::string getName();
    bool hasCountry(Country&);


};
#endif

Country.h:

#ifndef COUNTRY_H
#define COUNTRY_H
#include<string>
#include<vector>

class Map;
class Continent;

class Country{
    private:
        static int nextCountryNumber;
        int countryNumber;
        Map *map;
        std::string name;
        int x, y;
        Continent *continent;
        std::vector<Country*> adjacents;
    public:
        Country(std::string, int, int, Continent&, Map&);
        std::string getName();
        int getX();
        int getY();
        Continent getContinent();
        bool isAdjacentTo(Country&);
        bool hasAdjacent();
        void addAdjacent(Country&);
        std::string toString();
};
#endif

Answer 2

I think its some sort of circular referencing but I cant find where that is the case...

You have much worse than a circular referencing problem. You have a tangled mess. You could fix the circular referencing problem by following @personjerry's answer. This will not help solve your tangled mess problem.

The latter problem: Your class Country has data members of type Map and Continent and a vector of adjacent Country objects. Your class Continent has a vector of Country objects in that continent. Your class Map has vectors of Country and Continent objects. The Country named Foo in a Map object's countries data member is not the same object as the Country named Foo in a Continent object's countries data member. Even worse, all of your getters return copies. You have copies upon copies upon copies. This is a tangled mess.

The pre-C++11 way to solve this problem is to use pointers and to think very carefully of who owns what. In your case, the class Map appears to be primary as this is the class that is constructed from an input stream. The references to Continent and Map in class Country should be pointers or references. The vectors of Country objects in classes Country and Continent should be vectors of pointers.

The more modern approach is to use smart pointers (shared pointers in your case). One still needs to think a bit about who owns what, and that's very much the case here. Circular references present a bit of a problem with regard to shared pointers. You have a big potential for circular references with this structure.

Aside: Using using namespace std; is widely considered to be bad form. It is almost universally considered to be extremely bad form when that construct is in a header file.

Answer 3

For starters, in Map.h you need to forward declare classes Country and Continent

class Country;
class Continent;

before the declaration of class Map.

Answer 4

There's some issues in your code design. There are recursive container definitions.

The first main point to be aware of is that std::vector must have a complete type as the template parameter. This means you cannot have a class that contains a vector of itself; so you will have to rethink the design:

class Country
{
    // ...
    vector<Country> adjacents;

Similar comments apply to your other vectors: it's not sufficient to forward-declare a class and then declare a vector of it; you can't have vector<Country> inside class Map unless class Country is fully defined. Which it can't be, because class Country also contains a Map !

---

However, looking at the rest of your class and vector design, it looks more like what a Java or C# coder would do, where the container contains object references rather than objects.

In particular, Country containing a Map doesn't make much sense. A typical design would only have a single map, and the map contains many countries. But in your design, each country has its own map, completely separate to the map that any other country has (and completely separate to any global map).

In order to share references to a single instance amongst many users in C++, the right way is to use shared_ptr as a container for the object being referenced. (This also restricts how you allocate the objects - you must create them with make_shared rather than with direct declarations of the object).

I'm guessing you probably want to do the same thing with the Countries as well as the Maps. You'd rather have one Country object for each country, and various references to that country from its neighbours etc.

An alternative solution for Country requiring to hold a list of Countries would be to either hold a list of country ID/names that you look up when required; or to use a non-standard container which does permit declaration with incomplete type. The Boost Container library has some of these.