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?

`std::vector` has *very little* overhead over a raw array, and it's much safer and easier to use. — Angew is no longer proud of SO, Apr 15 '13 at 08:26
If I don't mistake, "Effective C++" says also that you should use `vector` in any cases, except you **exactly** (!!!) sure, that it will act worse then another container — borisbn, Apr 15 '13 at 08:41

score 3 · Accepted Answer · answered Apr 15 '13 at 08:35

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...

score 1 · Answer 2 · answered Apr 15 '13 at 08:29

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;
}

If she even can't tolerate the overhead of the pointer in std::vector, I wonder how she can stay with the idea of shared_ptr overhead and duplication of data ... — Emilio Garavaglia, Apr 15 '13 at 09:30
@EmilioGaravaglia - hmmmm.. didn't think about it... but I will )) — borisbn, Apr 15 '13 at 10:45

score 0 · Answer 3 · edited May 23 '17 at 12:26

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.

score 0 · Answer 4 · answered Apr 15 '13 at 09:29

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;
}

score 0 · Answer 5 · answered Apr 15 '13 at 09:37

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.

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

5 Answers5