Don't give away your internals? [C++]

Question

I am reading book called "C++ coding standard" By Herb Sutter, Andrei Alexandrescu and in chapter 42 of this book is an example:(chapter is short so I'm taking the liberty and pasting part of it)

Consider:

 class Socket {
 public:
   // … constructor that opens handle_, destructor that closes handle_, etc. …
   int GetHandle() const {return handle_;} // avoid this - (1) <-why this is bad code?
                                           // and why there is a comment to avoid such code??
 private:
   int handle_; // perhaps an OS resource handle
 };

Data hiding is a powerful abstraction and modularity device (see Items 11 and 41). But hiding data and then giving away handles to it is self-defeating, just like locking your house and leaving the keys in the lock. This is because:

Clients now have two ways to implement functionality: They can use your class's abstraction (Socket) or directly manipulate the implementation that your class relies on (the socket's C-style handle). In the latter case, the object is unaware of significant changes to the resource it thinks it owns. Now the class cannot reliably enrich or embellish functionality (e.g., proxying, logging, collecting statistics) because clients can bypass the embellished, controlled implementationand any of the invariants it thinks it's adding, which makes correct error handling next to impossible (see Item 70).

The class cannot change the underlying implementation of its abstraction because clients depend on it: If Socket is later upgraded to support a different protocol with a different set of low-level primitives, calling code that fetches the underlying handle_ and manipulates it incorrectly will be silently broken.

The class cannot enforce its invariants because calling code can alter state unbeknownst to the class: For example, someone could close the handle being used by a Socket object without going through a Socket member function, thus rendering the object invalid.

Client code can store the handles that your class returns, and attempt to use them after your class's code has invalidated them.

this is a summary from this book:

Don't volunteer too much: Avoid returning handles to internal data managed by your class, so clients won't uncontrollably modify state that your object thinks it owns.

Basically what I'm asking for is:

1. Why line marked by me as (1) is listed as an example of bad code (I always thought that returning pointers or reference is a bad idea but returning by value is OK. Here they're saying that returning by value is bad idea too?)

2. Is it possible that there is '&' missing and what they really mean is to not return internal data by reference or pointers?

Thank you.

Answer 1

I think what you're missing is that a handle — even though it's represented by an int in the type system — is a reference to something. This isn't returning some informational value — it's returning the object's internal reference to a system resource. The class should manage this handle itself, and the handle should be influenced by the external world only through the class interface.

Answer 2

The problem is not with the low level details (the code is perfectly good C++ that way).

The issue is that you are breaking your abstraction. What if in the future, instead of having an int handle, you need to change that to some sort of pointer. You would not be able to make that change without breaking any client that uses your class.

Answer 3

The point isn't that you're returning by value, which is fine, the point is that you're returning a resource handle.

Instead, your class should organize methods that access that resource and provide IO surrounding that resource.

If the resource is a file, for example, your class should have a write() and a read() method that reads and writes to/from the file.