Archived

This topic is now archived and is closed to further replies.

What the hell, I thought that was private!?

This topic is 5150 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I accidentally wrote this code and it compiles and actually runs but I dont know why. I thought that a private variable in C++ meant that no one could access that variable except for that class itself. c.h
#include <iostream>
using std::endl;
using std::cout;

class C {
public:
	C(int init);
	void doSomeShit(C *c);
private:
	int privateVariable;
};
c.cpp
#include "c.h"

C::C(int init){
	privateVariable = init;
}

void C::doSomeShit(C *c) {
	cout<<"this privateVariable: "<<this->privateVariable<<endl;
	cout<<"that privateVariable: "<<c->privateVariable<<endl;
	cout<<"what the hell, i thought that was private!"<<endl;
}
main.cpp
#include <iostream>
using std::cout;
using std::endl;

#include "c.h"

int main() {

	C *c1 = new C(111111);
	C *c2 = new C(222222);

	c1->doSomeShit(c2);

	return 0;
}
output
 
this privateVariable: 111111
that privateVariable: 222222
what the hell, i thought that was private!
Why can c1 access c2's private shit? [edited by - AndreTheGiant on November 10, 2003 10:15:45 AM]

Share this post


Link to post
Share on other sites
You may have used it before in your copy constructors to access the elements of another instance of the same class.

The C++ standard simply says:



A member of a class can be
— private; that is, its name can be used only by members and friends of the class in which it is
declared.
— protected; that is, its name can be used only by members and friends of the class in which it is
declared, and by members and friends of classes derived from this class (see 11.5).
— public; that is, its name can be used anywhere without access restriction.


[edited by - Jingo on November 10, 2003 10:30:29 AM]

Share this post


Link to post
Share on other sites
I still remember the day I learned this: about 3 months after using C++ - back then I was writing member functions like this:


MyClass& MyClass::operator=(const MyClass &rhs)
{
width = rhs.GetWidth();
height = rhs.GetHeight();
}

// instead of


MyClass& MyClass::operator=(const MyClass &rhs)
{
width = rhs.width;
height = rhs.height;
}

Share this post


Link to post
Share on other sites
It makes sense, you the class author are fully in control of all private violations between instances of your own class, so it doesn''t violate encapsulation. Java, and likely most OO languages, are the same way.

Share this post


Link to post
Share on other sites
quote:
Original post by tortoise
it doesn''t violate encapsulation.

Actually, it does.

But as always - it''s a judgement call. If you find yourself writing tons of getters merely as support for writing copy constructors, thats a far worse violation of encapsulation than just accessing the fields directly.

--
AnkhSVN - A Visual Studio .NET Addin for the Subversion version control system.
[Project site] [Blog] [RSS] [Browse the source] [IRC channel]

Share this post


Link to post
Share on other sites
quote:

It makes sense, you the class author are fully in control of all private violations between instances of your own class, so it doesn''t violate encapsulation. Java, and likely most OO languages, are the same way.



Yea, that does make a whole helluva lot of sense when you put it that way. Thanks!

Share this post


Link to post
Share on other sites
as the Arild Fines post mentions, there are cases where you want to use a function to access the member anyway ... but here''s the key ...

getter and setter functions are like any other function and may have (intentional) side effects ... so using or not using them, is just like calling any other internal helper function to do certain parts of the job ... and in some cases it''s correct (for instance if the setter provided profiling of how often the items interal value changed), but in other''s would be wrong (for instance if the setter provided logging of when the user requested such changes - I know this sounds silly, but just assume you are writing to a log to track usage of the public interface ... and the getter and setting are public ... you don''t want the log logging a "setter" call for each member set during a constructor do you ... not usually) ..

so like always, you have to think sometimes even to make this silly little decision ...

usually I access values directly, UNTIL I add side effect code to the getter/setter methods ... then at that point (and not before) I perform a code audit through each use or set of the value, and change some to use the setter ... but this has only really happened to me twice ... so that''s why I don''t use them to start with, why spend work up front to save on maintenece you most likely won''t have anyway ... if it becomes a maintence issue, use the refactoring pattern arild refered to ... but only IF it becomes a factor - don''t assume it will before hand.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
using get/set methods that are inlined is the same thing as accessing the variable directly.

ie:

class myclass
{
int x;
public: inline void getX() { return x; }
public: void doSomething()
{
x = 10; // same as...
getX() = 10; // this
}
};

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
wooops major typo by me AP above, the get method should look like this:


inline int& getX() { return x; }

Share this post


Link to post
Share on other sites
Aww man! That kinda sux. Take a look at this:

class C{
private:
int *arr, len;
public:
C(){ len=0; malloc(arr,sizeof(int)*len); }
void add(int i){
len++;
realloc(arr,sizeof(int)*len);
arr[len-1]=i;
}
void RuinItAll(C *c){
realloc(c->arr,sizeof(int)*1);
}
};


"On a long enough timeline the survival rate of everyone drops to zero"
- Fight Club

Share this post


Link to post
Share on other sites
However - getter and setter functions would only be in the way in this case:

class C{
private:
string s;
public:
C(string s){ this->s=s; }
inline void SetS(string s){ this->s=s; }
inline string GetS(){ return s; }
};

Nothing else is done in the getter / setter than setting / getting the target variable. In this case I would just make the variable public and throw away the getter and setter:

class C{
public:
string s;
C(string s){ this->s=s; }
};

Share this post


Link to post
Share on other sites
ALWAYS make sure your getters are const ... then you won''t make thats mistake ...


inline int& getX() { return x; }
// should be const

inline int& getX() const { return x; }
// is invalid for const reasons ...

// so only these two
inline const int& getX() const { return x; }
inline int getX() const { return x; }
// are valid


and remember, for primitive types it is more efficient to return the value than the reference ... and nobody''s syntax using it needs to change, so choose the second option above for this case.

Share this post


Link to post
Share on other sites
quote:
inline const int& getX() const { return x; }inline int getX() const { return x; }// are valid


and remember, for primitive types it is more efficient to return the value than the reference ... and nobody's syntax using it needs to change, so choose the second option above for this case.


Yes, but as was discussed in another thread recently, the const-ness can be cast away by a well-meaning programmer that's trying to be clever:

const_cast<int &>(f.geti()) = 666;


I like your final suggestion - return a copy of the int, not a reference (even a const one) to it.

As an aside, if you notice that certain classes are always twiddling with the private members of another class, it's a good sign that a refactoring is in order. Perhaps the "twiddler" and "twiddlee" need to be merged.

EDIT: Stupid source tags!

--
Dave Mikesell Software & Consulting

[edited by - dmikesell on November 12, 2003 10:31:50 AM]

Share this post


Link to post
Share on other sites
the fact that constness can be cast doesn''t affect anythign ...

one the one side casting the constness away inside the function, this allows one to internalally do things which are not const (effectively lying to the user - but since this code is in the class, that is within your rights), the better solution for that is use mutable for the members that may be changes in const functions ... such as access counts or profiling objects.

casting away const in the client (caller) of the function is also not an issue, as that caller has raw memory access into your object as well ... so they can crash anything they want to if they so choose, but that is beyond your domain to worry about ... If you are worried about hacker like in your example, you must first realize that they have accesss to yhe header file, and therefore complete knowledge about the memory layout of your object ... so any protections of access in C++ are only for user''s on good behavior ... not actual security level protections.

the point of const, is that the compiler doesn''t allow users to modify const objects WITHOUT KNOWING they are modifying the object.

Share this post


Link to post
Share on other sites