How to avoid returning handles to object internals - Item 28 Effective C++

Question

Item 28 of Effective C++ says avoid returning "handles" to object internals. This question shows how to design your code to do exactly that by thinking about encapsulation in order to avoid accidentally exposing the internals of your class.

My example involves a data array and since memory is an issue I'd like to avoid using std::vector (and the Boost library).

Using an array here's a very simplified version of my code:

class Foo {
public:
    Foo(int size) : data_(new int[size]) {
        // Populate with data just for the example
        for(int i = 0; i < size; i++) {
            data_[i] = i;
        }
    }

    // I know that this isn't good practice
    int* const get_data() const {
        return data_;
    }

    ~Foo() {
        delete[] data_;
    }

private:
    int* data_;
};

int main(void) {
    Foo* foo = new Foo(10);
    int* const data = foo->get_data();
    delete foo;
    // data now dangles which is bad!
    return 0;
}

I understand that using const with get_data() doesn't make it safe. If I was using a vector I could copy it like the example question above, but since I'd like to avoid this I was wondering how best to design my class to avoid this potentially dangerous situation?

Answer 1

Programming and software design is a constant "choice of one thing over another". If you REALLY can't afford using std::vector, and you need to expose data to the client-code using your class, then so be it. Document why it is so, and make sure that it's reviewed over time - it may be that when your new model of hardware that has 8MB of RAM instead of 2MB will be able to use the extra 4 bytes that a vector takes over your plain pointer.

Or write an accessor function:

int& Foo::operator[](int pos)
{
   // Add checks here to ensure `pos` is in range?
   return data[pos]; 
}

Now you don't have to return the pointer at all...

Answer 2

The one way in your situation is to use smart pointers. I'd prefere shared_ptr. Here's how you can modify your class to avoid problems you raise:

class Foo {
public:
    Foo(int size) : data_(new int[size]) {
        // Populate with data just for the example
        for(int i = 0; i < size; i++) {
            data_[i] = i;
        }
    }

    // I know that this isn't good practice
    std::shared_ptr<int> const get_data() const {
        return data_;
    }

    ~Foo() {
        //delete[] data_;
    }

private:
    std::shared_ptr<int> data_;
};

int main(void) {
    Foo* foo = new Foo(10);
    std::shared_ptr<int> data = foo->get_data();
    delete foo;
    // data still good )) // --now dangles which is bad!--
    return 0;
}

Answer 3

There are plenty of cases where returning pointers is perfectly acceptable, even the array was actually contained in std::vector; e.g. using external APIs and when copying data is not appreciated.

I don't see the danger since you're not programming for monkeys. User of the class ought know. But I added function to query the size of the data. That is for safety.

extern "C"{
void foobar( const int *data, unsigned int size );
}
class Foo {
public:
    Foo(unsigned int size) : size_(size), data_(new int[size]) {
        // Populate with data just for the example
        for(unsigned int i = 0; i < size; i++) {
            data_[i] = i;
        }
    }

    unsigned int get_size() const{return size_;}
    int* const get_data() const {
        return data_;
    }

    ~Foo() {
        delete[] data_;
    }

private:
    int* data_;
    unsigned int size_;
};

int main(void) {
    Foo* foo = new Foo(10);
    int* const data = foo->get_data();
    foobar( data, foo->get_size() );
    delete foo;
    return 0;
}

Answer 4

considering even the standard library has get_native_handle() on classes like std::thread I wonder who between the standard library or EC++ was wrong!

Effective C++ is a set of guidelines you simply can't apply them all in any context. It's a "moral" book. Don't treat it as "legal". If you are required to expose the data just do it.

After all who deletes a pointer to your internal data (after getting it through a getter) is also doing a "bad thing", since he's acting destructively on something does not belong to him.

Answer 5

If you really wish to avoid using std::vector, you can implement approach similar to one used by some winapi functions.

size_t get_data(int* data_ptr, size_t& buffer_size)
{
if(buffer_size < sizeof (data_)
{
buffer_size = sizeof(data);
return 0;
}
//You could you memcpy for ints here
        for(int i = 0; i < size; i++) {
            data_ptr[i] = i;
        }
return sizof(data_);
}

Thus requiring caller to provide memory to store data and care about allocations of that memory.
Thru it should be reinvention of what std::vector already would do with much less safety. Also you should check Rule of three to avoid problems when you will start using your class more extensively.