C++

Forum

 
Friendship not recognized. Getting cannot access private member in class 'Account'

theperson (59)
I have declared createAccount a friend in Account.h but I'm not able to to directly cout a member variable in account.h from within createAccount


MainMenu.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
#include "SimpleVector.h"
#include "Account.h"

using namespace std;



class MainMenu
{
private:

public:
	MainMenu(void);

	//menu options
	void createAccount();


	~MainMenu(void);


};



MainMenu.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
#include "MainMenu.h"



//Constructor
MainMenu::MainMenu(void)
{
}

void MainMenu::createAccount()
 {
	 Account newAccount;

	 cout << newAccount.firstName;

     
 }



//Destructor
MainMenu::~MainMenu(void)
{
}




Account.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
#ifndef ACCOUNT_H
#define ACCOUNT_H

#include <string>


using namespace std;

class Account
{
	private:
		string firstName;	
		

	public:
		//constructor
		Account(void);


		//friends
		
		friend void MainMenu::createAccount();



		//destructor
		~Account(void);
};

#endif
TwilightSpectre (1392)
'Account.h' doesn't know of the existance of MainMenu::createAccount(). Instead, #include "mainmenu.h" from 'account.h' and rather than including 'account.h' from 'mainmenu.h', just forward declare it (no more than that is needed). Also, please don't have using namespace std;, or even using namespace anything inside header files: It forces that namespace on someone, who may not have necessarily wanted it. Instead, either scope your namespace declarations or just fully type everything (its only 5 characters, after all).

Here is a working example (one file):
http://ideone.com/UVH6CU
theperson (59)
Include "MainMenu.h" in "Account.h" but don't include "Account.h" in "MainMenu.h"?


When I do this...


Account.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
 
ifndef ACCOUNT_H
#define ACCOUNT_H

#include <string>
#include "MainMenu.h"


using namespace std;

class Account
{
	private:
		string firstName;	
		

	public:
		//constructor
		Account(void);


		//friends
		
		friend void MainMenu::createAccount();



		//destructor
		~Account(void);
};

#endif



MainMenu.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
#pragma once
#include <iostream>
#include <fstream> //for reading accounts into array

#include "SimpleVector.h"


using namespace std;


class MainMenu
{
private:

public:
	MainMenu(void);

	//menu options
	void createAccount();


	~MainMenu(void);


};




I get 

1
2
3
4
 
error C2653: 'MainMenu' : is not a class or namespace name
error C2248: 'Account::firstName' : cannot access private member declared in class 'Account'

etc.


I know using namespace std is inefficient but my instructor doesn't care and it makes things easier
ne555 (10692)
> #include "SimpleVector.h"
¿what's that and why does `MainMenu' need it?

> rather than including 'account.h' from 'mainmenu.h', just forward declare it (no more than that is needed)
not even that is needed.

> I know using namespace std is inefficient
it is not inefficient.
Last edited on
theperson (59)
I broke down the problem to make it easier for people on here to compile and look at. SimpleVector was just an include that I forgot to delete

Forward declare "account.h"? Is that possible even?

It isn't inefficient? So it just makes errors more likely to occur or something?
Jaybob66 (534)
It isn't inefficient? So it just makes errors more likely to occur or something?


it makes no difference to compiled code. a namespace is just an abstract wrapper so that function names don't collide with names in other libraries you may be using.

if you are using 2 libraries and they both have a sort function that takes the same parameters you can distinguish them with lib1::sort() and lib2::sort()

you can forgo the namespace name if you know there are no clashes.
Last edited on
ne555 (10692)
> I broke down the problem to make it easier
¿did you make sure that your snip reproduces the problem?

1
2
3
4
5
6
7
8
9
10
11
12
 
//MainMenu.h
#pragma once

class MainMenu
{
	private:
	public:
		MainMenu(void);
		//menu options
		void createAccount();
		~MainMenu(void);
};
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
//Account.h
#pragma once
#include <string>
#include "MainMenu.h"

class Account
{
	private:
		std::string firstName;	
	public:
		//constructor
		Account(void);
		//friends
		friend void MainMenu::createAccount();
		//destructor
		~Account(void);
};
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
//MainMenu.cpp
#include "MainMenu.h"
#include "Account.h"
#include <iostream>
//Constructor
MainMenu::MainMenu(void)
{
}

void MainMenu::createAccount()
{
	Account newAccount;
	std::cout << newAccount.firstName;
}

//Destructor
MainMenu::~MainMenu(void)
{
}
theperson (59)
Yeah, I did make sure to reproduce the problem. Ok the program does work when I include "Account.h" in MainMenu.cpp and it does not work when I include "Account.h" in "MainMenu.h" but I was under the impression that I should only include MainMenu.h in MainMenu.cpp. I thought that was a kind of convention to include everything extra that needs to be included in the .h file rather than the .cpp file.

EDIT: Actually... no I think I didn't reproduce the problem. :( The fix for the snippet doesn't seem to work in the original code. I'll have to try again
Last edited on
Disch (13742)
FWIW, this seems like a misuse/abuse of friendship. There does not seem to be a [good] reason for MainMenu to be a friend.

If you're going to use friendship for every function for which you want to have access to private vars... you might as well just not make the vars private. It defeats the point.
theperson (59)
There is actually a good reason for it in my actual program. I want createAccount to be able to store values in the private members of account and they need to be validated first. If createAccount is a friend, then I can do something like this

1
2
3
 
//prompt
cin >> theAccount.firstName
theAccount.fistName = validateNoSpacesAndNotEmpty(theAccount.firstName);


Rather than this

1
2
3
4
5
6
 
string firstName;
//prompt
cin >> firstName;
firstName = validateNoSpacesAndNotEmpty(firstName);
theAccount.setFirstName(firstName);



I have to do this many times for different member variables so it would make the code a lot shorter
theperson (59)
This is my actual code, rather than the broken down problem


MainMenu.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
 
#ifndef MAINMENU_H
#define MAINMENU_H

#pragma once
#include <iostream>
#include <fstream> //for reading accounts into array

using namespace std;

#include "menu.h"
#include "SimpleVector.h"
#include "Validation.h"




class MainMenu :
	public Menu
{
private:
	
	int accountArraySize;

	//Account* appendToAccountArray(Account newAccount, Account accountArray[]); 
	

public:
	MainMenu(void);

	//menu options
	void createAccount(Account accountArray[]);

	void getUserLoggedIn();

	void showAllAccounts();

	void exitSystem();


	
	void readAccountsIntoArray(fstream &allAccountsFile, Account accountArray[]);


	//overriding the virtual functions
	void displayMenu();

	void runMenuSystem();

	~MainMenu(void);



};

#endif



MainMenu.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
 
#include "MainMenu.h"
#include "Account.h"


//Constructor
MainMenu::MainMenu(void)
{
}

Account* MainMenu::appendToAccountArray(Account newAccount, Account accountArray[])
{
	//new Account newAccountArray[];
}


void MainMenu::createAccount(Account accountArray[])
 {
	 Account newAccount;

cout << "Please enter the following information to create an account.\n\n";

     
 }


void MainMenu::getUserLoggedIn()
{

}

void MainMenu::showAllAccounts()
{

}

//virtual Functions
void MainMenu::displayMenu()
{
	cout << "\n\n\t1. Create an account\n"
		 << "\t2. Log in\n"
		 << "\t3. Show all accounts\n"
		 << "\t4. Exit System\n"
		 << "\n\n";

}

void MainMenu::exitSystem()
{
	exit(0);
}

void MainMenu::readAccountsIntoArray(fstream &allAccountsFile, int &arraySize, Account accountArray[])
{

	//This needs to be a char array rather than a string so it will work
	//with conversion functions atoi and atof which are needed to set the account
	//number and balance. buffer temporarily holds a data
	//item read from allAccountsFile.
	char buffer[50];

	//using this instead of buffer when setting value of strings in account
	//so that I won't have to convert from charArray to string
	string tempString;

	//read account info from the file for all the accounts and
	//store the info in the member variables of all the accounts
	for (int i = 0; i < arraySize; i++)
	{
		
		allAccountsFile >> buffer;
		accountArray[i].setAccountNumber(atoi(buffer)); //atoi converts from char array to int
		allAccountsFile >> tempString;
		accountArray[i].setFirstName(tempString);
		allAccountsFile >> tempString;
		accountArray[i].setLastName(tempString);
		allAccountsFile >> tempString;
		accountArray[i].setPassword(tempString);
		allAccountsFile >> buffer;
		accountArray[i].setBalance(atof(buffer));

		//reason for using the buffer and tempString is that something like this won't work
		//because the setters take an argument
		//allAccountsFile >> accountArray[i].setBalance();
	}

	allAccountsFile.close();

}




void MainMenu::runMenuSystem()
{
	//minimum and maximum integers that are valid menu choices
	//will pass these to validateMenuChoice
	int minChoice = 1;
	int maxChoice = 4;

	displayMenu();
	int menuChoice = 0;
	menuChoice = validateMenuChoice(menuChoice, minChoice, maxChoice);

	fstream allAccountsFile;
	
	//will hold the account array
	Account* accountArray;



	//will create an array of the correct size and read the accounts into it so that all the the menu options
	//will work

	//will hold number representing the size the array should be so that it can be converted
	//to an integer with atoi
	char buffer[50];

	//open fstream for input
	allAccountsFile.open("Accounts.txt", ios::in);

	if (!allAccountsFile)
	{
		cout << "Error opening file with account information for input";
	}
	else
	{

		//read in initial size for array
		allAccountsFile >> buffer;
	
		//set member variable accountArraySize to integer read from the file
		accountArraySize = atoi(buffer);

		accountArray = new Account[accountArraySize];

		readAccountsIntoArray(allAccountsFile, arraySize, accountArray);



		//execute different functions depending on users menu choice
		switch (menuChoice)
		{
			case 1: 
				{
					createAccount(accountArray);
					break;
				}
			case 2:
				{
					getUserLoggedIn();
					break;
				}
			case 3:
				{
					showAllAccounts();
				}
			case 4:
				{
					exitSystem();
				}
		
		}
	}
}


//Destructor
MainMenu::~MainMenu(void)
{
}





Account.h
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
 
#ifndef ACCOUNT_H
#define ACCOUNT_H

using namespace std;
#include <string>

#include "SimpleVector.h"
#include "MainMenu.h"


class Account
{
	private:
		
		string firstName;		
		string lastName;
		int accountNumber;
		string password;
		double balance;
		
		int numTransactions;
		
		SimpleVector<double>transactionVector();
		
		

	public:
		//constructor
		Account(void);


		//member functions
		void makeWithdrawal(double amount);

		void makeDeposit(double amount);

		void writeCheck(double amount);

		double getTransactionAmount(int transactionNumber);

		bool validateLogin(string pWord, int accountNumber);

		void logOut();


		//friends
		
		friend void MainMenu::createAccount(Account accountArray[]);


		//setters
		 void setFirstName(string f);

		 void setLastName(string l);

		 void setAccountNumber(int a);

		 void setPassword(string p);

		 void setBalance(double b);

		 void setNumTransactions(int n);


		 //getters
		string getFirstName();

		string getLastName();

		int getAccountNumber();

		string getPassword();

		double getBalance();

		int getNumTransactions();

	



		//destructor
		~Account(void);
};

#endif





Account.cpp
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
 
#include "Account.h"

//Constructor
Account::Account(void)
{
}


//setters
void Account::setFirstName(string f)
{	
	firstName = f;
}	

void Account::setLastName(string l)
{	
	lastName = l;
}	

void Account::setAccountNumber(int a)
{	
	accountNumber = a;
}	

void Account::setPassword(string p)
{	
	password = p;
}	

void Account::setBalance(double b)
{	
	balance = b;
}	

void Account::setNumTransactions(int n)
{	
	numTransactions = n;
}	


//getters
string Account::getFirstName()
{
	return firstName;
}

string Account::getLastName()
{
	return lastName;
}

int Account::getAccountNumber()
{
	return accountNumber;
}

string Account::getPassword()
{
	return password;
}

double Account::getBalance()
{
	return balance;
}

int Account::getNumTransactions()
{
	return numTransactions;
}



//destructor
Account::~Account(void)
{
}
Last edited on
theperson (59)
I'm getting tons of errors

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
1>  Source.cpp
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(31): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(41): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\account.h(48): error C2245: non-existent member function 'MainMenu::createAccount' specified as friend (member function signature does not match any overload)
1>  Menu.cpp
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(19): error C2504: 'Menu' : base class undefined
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(31): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(41): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\account.h(48): error C2245: non-existent member function 'MainMenu::createAccount' specified as friend (member function signature does not match any overload)
1>  MainMenu.cpp
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\account.h(48): error C2653: 'MainMenu' : is not a class or namespace name
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.cpp(10): error C2039: 'appendToAccountArray' : is not a member of 'MainMenu'
1>          c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(17) : see declaration of 'MainMenu'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.cpp(79): error C2511: 'void MainMenu::readAccountsIntoArray(std::fstream &,int &,Account [])' : overloaded member function not found in 'MainMenu'
1>          c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(17) : see declaration of 'MainMenu'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.cpp(162): error C2065: 'arraySize' : undeclared identifier
1>  Account.cpp
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(31): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\mainmenu.h(41): error C2061: syntax error : identifier 'Account'
1>c:\users\alexander\documents\visual studio 2012\projects\midterm_project\midterm_project\account.h(48): error C2245: non-existent member function 'MainMenu::createAccount' specified as friend (member function signature does not match any overload)
Disch (13742)
I'm coming at this from a standpoint of encapsulation. Newbies are told "don't make your data members public", but don't really understand the reason why that's generally bad. Nor do they really grasp the concept of encapsulation and how it's used.

Feel free to disregard my advice and do things your own way. I'm posting merely to try and give you insight as to why a real-world program would not be written this way.




The whole point to encapsulation is that you keep specific classes isolated so that no outside code needs to understand their inner workings. Instead, a simplified interface of abstracted concepts is given to outside code.

When a class is properly encapsulated, it makes it difficult to "misuse". Compare the dangers of using raw char arrays for strings vs. the std::string class. With char arrays you need to be mindful of buffer overflows, memory management, making sure your strings are null terminated, etc, etc. But with std::string, all of that is handled "under the hood", and you are given a simple, intuitive interface to interact with the class.

All of this outside code can now use std::string because of its simple interface... and none of it has to care about how the class works internally. As long as it works as described... the means by which it accomplishes it doesn't matter.

This is a powerful concept in programming because it allows for code reusability, easier testing, easier debugging, and just in general improved organization of the code and logic.

This is why you keep your data members private. If they're private you have full control over the state of the object. It keeps the class's code isolated. If other areas of the code and stick their dirty little fingers in and muck up the data, then it's difficult/impossible to ensure the class will always work.



By contrast... if you have public members (or a dozen friends), then your class is no longer encapsulated. If you start having problems with your Account class... now you don't know whether the problem is coming from your Account code or your MainMenu code, or your <insert other friend here> code.

If the class was encapsulated... you'd know the problem was in your Account code because that's the only code that can access it. It makes the problem much easier to find and fix.

There is actually a good reason for it in my actual program. I want createAccount to be able to store values in the private members of account and they need to be validated first


Input validation could be done in the setter. That's kind of what setters are for. If a setter is just going to blindly assign a data member even if what is being assigned is wrong... then you're violating encapsulation and once again, there is no point in using getters/setters and you might as well just make the data public to begin with.

The whole point with having encapsulation is that no data members are ever invalid in the object. You keep everything private, and everything valid... so that you know the object is always in a good state. Or at least in an expected state.

If you let MainMenu access Account's members, Account can no longer guarantee that it is always going to be valid, because it's at the mercy of MainMenu.



EDIT: Also... instead of MainMenu getting the input... Account could be doing it. Friendship should only be used between very tightly-knit classes. Classes that cannot exist without one another.

Here... there is no reason for Account to be bound to MainMenu, so there is no reason for them to be friends.
/EDIT

I have to do this many times for different member variables so it would make the code a lot shorter


So why not remove the getters/setters entirely and make the data public? That's even shorter code. </rhetorical. I'm not actually trying to get you to do this.. just pointing out a hole in your logic>

Encapsulation is not about writing shorter code. It's about writing stronger code.
Last edited on
theperson (59)
Thanks very much for the detailed response. I hadn't even thought of doing validation in the setters and didn't entirely understand why encapsulation was more secure. I just figured it looks more organized and it being secure was something I was taking on faith more or less. I will call validateNoSpacesAndNotEmpty() from within the setters instead.
Topic archived. No new replies allowed.