Hi everyone,

I'm a moderate-level C++ programmer who is a little rusty at the moment. I've got an object question which is driving me nuts. I'm sure this is a C++ 101 level question, but for the life of me, I can't recall the solution. Basically, I've got one object which has to access private data in another object... and can't.

Here's the specifics: I'm writing a little war game program where players deploy units (soldiers, tanks, planes, etc.) onto a gameboard. Players and Units are modeled as objects:

class GameUnit {
  public:
    string GetName()  {return Name;}
  protected:
    string Name;
};


class Player {
  public:
    void ListUnits();
  protected:
    vector<GameUnit*> MyUnits;
};

void Player::ListUnits() {
  for(unsigned int i=0; i<MyUnits.size(); i++) {
    cout<<MyUnits[i]->GetName()<<"\n";
  }
}

Here's the problem: In the above code, Player's ListUnits() function doesn't work because Player can't access GameUnit's GetName() function. Specifically, here's the compiler's error message:

In file included from Main.cpp:18:
Player.h: In member function 'void Player::ListUnits()':
Player.h:47: error: 'GetName' undeclared (first use this function)
Player.h:47: error: (Each undeclared identifier is reported only once for each function it appears in.)

I've tested enough to realize that the problem is the GameUnit::GetName() function is a public function within the GameUnit object. Why can't a Player call this function? Making both friend classes of each other doesn't help. Do I have to make these guys related somehow?

I'm sure this is a fairly simple problem, but I can't find the solution for the life of me.

Many thanks,
-P

I tried this and my compiler didn't complain.

#include <iostream>
#include <vector>
#include <string>

using namespace std;

class GameUnit {
  public:
    string GetName()  {return Name;}
  protected:
    string Name;
};


class Player {
  public:
    void ListUnits();
  protected:
    vector<GameUnit*> MyUnits;
};

void Player::ListUnits() {
  for(unsigned int i=0; i<MyUnits.size(); i++) {
    cout<<MyUnits[i]->GetName()<<"\n";
  }
}

int main()
{
  return 0;
}

Post the entire program because the code snippet you posted compiles ok for me using vc++ 2010 express compiler/IDE.

[edit]I used the exact same code as ^^^ posted.

Edited 4 Years Ago by Ancient Dragon: n/a

Hi everyone,

Thanks for taking a gander. Below is the full code, a few unrelated things omitted. I've also included the makefile I'm using for good measure. (I'm working on a VMware machine, emulating Solaris.)

Yours,
-P

#------------------------------------------------------------------------------
# File: Makefile
#
# Note: This Makefile requires GNU make.
#
# (c) 2001,2000 Stanford University
#
#------------------------------------------------------------------------------

CC = g++

CFLAGS = -g -Wall -ansi -pg -D_DEBUG_ $(ARCH)
#CFLAGS = -g -Wall -Werror -ansi -pg -D_DEBUG_ $(ARCH)

LIBS =

SRCS = 

all: exe

exe: $(SRC)
	$(CC) $(CFLAGS) $(LIBS) Main.cpp -o exe


.PHONY : clean clean-deps

clean:
	rm -f *.o *~ core sr *.dump *.tar tags

clean-deps:
	rm -f .*.d


//<><><><><><><><><><><><><><><><><><><><><><><><><><>
//<><><><><><><><><><><><><><><><><><><><><><><><><><>


//"Player.h"

class GameUnit;

class Player {

  public:

    //Constructors

      Player(int MyID, string MyFactionName);

      ~Player();



    //Accessors

      friend class GameUnit;
      int  GetID()  { return ID; }


      string GetFactionName()  { return FactionName; }

      void AddUnit(GameUnit* NewUnit) {MyUnits.push_back(NewUnit);}
      void ListUnits();
      void ListUnitsByPtr();
      Player* FindPlayer(string MyFactionName);

    // Other



  protected:
    int ID;
    string FactionName;
    vector<GameUnit*> MyUnits;

  };





Player::Player(int MyID, string MyFactionName) {
  ID=MyID;
  FactionName=MyFactionName;
}


Player::~Player() {
  // do nothing
}


void Player::ListUnits() {
  if(MyUnits.size()>0) {
    for(unsigned int i=0; i<MyUnits.size(); i++) {
      cout<<MyUnits[i]->GetName()<<", ";
    }
    cout<<"\n";
  }
  else cout<<"No Units!\n";
}


void Player::ListUnitsByPtr() {
  if(MyUnits.size()>0) {
    for(unsigned int i=0; i<MyUnits.size(); i++)
      cout<<MyUnits[i]<<", ";
    cout<<"\n";
  }
  else cout<<"No Units!\n";
}


//<><><><><><><><><><><><><><><><><><><><><><><><><><>
//<><><><><><><><><><><><><><><><><><><><><><><><><><>


//"GameUnit.h"


// Enums

enum GameUnit_int
  { Attack, Defend, Cost, Move, Damage, LAST_VALUE_INT };
enum GameUnit_bool
  { JetPower, SuperSubs, LRAircraft, HeavyBombers, LAST_VALUE_BOOL };



class GameUnit {

  public:

    //Constructors

      GameUnit(string MyName, int a, int b, int c, int d, int e);

      ~GameUnit();



    //Accessors

      friend class Player;
      string GetName()                 {return Name;}

      void SetInt(int Index, int a)    {MyInts[Index]=a;}
      int  GetInt(int Index)           {return MyInts[Index];}
      void AddToInt(int Index, int a)  {MyInts[Index]+=a;}

      void IncrementInt(int Index)     {MyInts[Index]+=1;}

      void DecrementInt(int Index)     {MyInts[Index]-=1;}


    // Other



  protected:
    int MyInts[LAST_VALUE_INT];

    //int Attack, Defend, Cost, Move, Damage;
    bool MyBools[LAST_VALUE_BOOL];
    string Name;

  };





GameUnit::GameUnit(string MyName, int a, int b, int c, int d, int e) {

  cout << "GameUnit constructor!\t";

  Name=MyName;
  MyInts[Attack]=a;  MyInts[Defend]=b;  MyInts[Cost]=c;
  MyInts[Move]=d;    MyInts[Damage]=e;
  for(int i=0; i<LAST_VALUE_BOOL; i++)  MyBools[i]=0;
  cout << "Finished creating a "<<Name<<"\n";

}





GameUnit::~GameUnit() {

  //do nothing

}




//<><><><><><><><><><><><><><><><><><><><><><><><><><>
//<><><><><><><><><><><><><><><><><><><><><><><><><><>


#include <map>

#include <iostream>

#include <sstream>

#include <fstream>

#include <time.h>

#include <vector>

#include <algorithm>

#include <queue>

#include <cstring>

#include <string>

#include <math.h>

#include <stdlib.h>

#include <cstdio>

#include <time.h>

#include <map>

using namespace std;



#include "Player.h"
#include "GameUnit.h"



int main() {


  PlayerManager MyPlayerManager;
  PlayerManager* PtrMyPlayerManager = &MyPlayerManager;

  PlayGame(PtrMyPlayerManager);


  return 0;

}

Sorry, I should have removed the bulk of the main() function. For the purposes of this thread, it should be:

int main() {
  return 0;
}

lines 173 - 194: Do not put executable code inside header files, move those two functions into a *.cpp file like GameUnit.cpp. The same goes for Player.h.

After making the above changes everything compiled ok for me. I just added two more files: GameUnit.cpp and Player.cpp

Edited 4 Years Ago by Ancient Dragon: n/a

Your codes are not bad, but there are still something could be refine

//this is not initialize but assignment
Player::Player(int MyID, string MyFactionName) {
  ID=MyID;
  FactionName=MyFactionName;
}
//This is initialize
Player::Player(int MyID, string MyFactionName) :ID(MyID), FactionName(MyFactionName){ }
//you are intent to copy the string, use std::string const& instead of string
//if you want to construct MyFactionName directly, it is another story
Player::Player(int MyID, string MyFactionName) {
  ID=MyID;
  FactionName=MyFactionName;
}
//you could save the size into another variable
//size_t Size = MyUnits.size();
//use size_t rather than unsigned int, it would be easier to shift to 64bits
//use prefix++ rather than postfix++
//use != rather than < if you want to cope with generic programming
for(size_t i=0; i != Size; ++i) {

just some opinions of coding style, another problems like protected or the name
like a, b, c, d, e.Better don't do these in a big project or when you need to
cooperate with other.

If my comments have any mistakes, please feel free to critics me.Thanks

Awesome, these are great tips. Thanks to your feedback, I was able to zero in on the real problem, the order of the included files was complicating matters. I've since rebalanced, and things now look great.

Many thanks!
-P

This question has already been answered. Start a new discussion instead.