user487100
user487100

Reputation: 918

How to cope with lifetime of returned pointer?

I am making a set of classes to represent an image. One of the applications of this class will be to draw a picture over a set of tiled images. The abstract image class looks something like this:

class Image
{
public:
    virtual Pixel* findPixel( Point p ) =0;
    virtual bool isDrawable( Point p ) =0;
    virtual bool contains( Point p ) =0;
};

The problem I forsee is that if I make a class like this:

class TiledImage : public Image
{
    std::vector<Image*> tiles;

public:
    Pixel* findPixel( Point p )
    {
        // find the tile that contains the point.
        // ask it for the pixel that contains the point.
        // return the pixel.
    }

    // etc....

};

that is designed to create, save, and delete subsections (tiles) of a very large image as needed, then it is possible the user could store a pointer to a Pixel object that may eventually not exist anymore.

One option is require the user to check the pixel back in when they are done, like:

Pixel* p = image.findPixel( aPoint );
// do stuff
image.returnPixel( p ); // p is not guaranteed to be valid after this point.
p->doSomething(); // this is not guaranteed to work.

I don't really like this because if the user doesn't return the pixel, then it could really mess with the operation of the tiled image -- forgetting to return a pixel could cause it to come to a complete lockup as it won't be able to delete tiles that aren't needed anymore. They will be locked to guarantee the pointer to the pixel remains valid. The reason for the lockup might be difficult for the user to discover.

In addition, this concern is kind of specialized. You wouldn't expect the pixel to disappear until the image disappears in a typical case.

Is there a better way to handle this situation? Smart pointers? Don't return a reference at all somehow? Does making TiledImage inherit from Image not make sense in the first place? I sure would like to be able to pass a TiledImage in as an Image when I expect the graphic to be very large.

Thanks.

Upvotes: 1

Views: 128

Answers (3)

hmjd
hmjd

Reputation: 121961

Two possibilities (at least):

  • use shared_ptr (either boost or std):

    std::vector<shared_ptr<Image>> tiles; // No longer have to explicitly
                                          // delete the elements.
    
    shared_ptr<Pixel> findPixel( Point p )
    {
        ...
    }
    
    shared_ptr<Pixel> p = image.findPixel( aPoint ); 
    p->doSomething(); // Fine, as the internal pointer will not be
                      // deleted due to use of the smart pointer.
    
  • return a copy of the Pixel. This would only be viable if a Pixel is always expected to be found or if there is a default value available that could be returned to indicate non-existence.

Minor point, all the member functions imply they do not change Image so consider making them const.

Upvotes: 5

Pubby
Pubby

Reputation: 53037

Iterators for things like std::vector obviously are invalidated when the vector is destroyed, I don't see how this is much different than your case.

My recommendation is to return the pixel by reference as that clearly shows that it's not handing over ownership. Also, provide good documentation on the behavior of the invalidation.


There have been recommendations for shared_ptr, and if you take that route it seems like a good decision to return a weak_ptr instead of a shared_ptr as the ownership belongs to the image.

Upvotes: 1

smerlin
smerlin

Reputation: 6566

RAII is definetly the way to go here. Your Pixel class should call image.returnPixel(this) in its destructor and findPixel should return a shared_ptr<Pixel>. If the pixel may not be deleted once the user is done, you could instead return a shared_ptr with a custom deleter which does not delete the Pixel, but calls returnPixel instead.

Upvotes: 1

Related Questions