0

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

4
Contributors
7
Replies
8
Views
5 Years
Discussion Span
Last Post by phummon
0

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;
}
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 by Ancient Dragon: n/a

0

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;

}
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;
}
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 by Ancient Dragon: n/a

0

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

0

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.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.