Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a class that represents a file in specific binary format on disk (the parent in the title) and another class that represents an object inside that file (child). When the object changes, it sometimes needs to relocate to a new position in the file and the file class has to be notified about this new position.

To do this, I have added a weak_ptr field to object, which points to the file. Except that means there has to be a shared_ptr for file, so I'm using a static factory method instead of public constructor for it. And object now can't be initialized in the constructor, so I'm using unique_ptr for it.

In code, it looks something like this:

class File;

class Object
{
private:
    std::weak_ptr<File> file;

public:
    Object(std::weak_ptr<File> file)
        : file(file)
    {}

    void modify();
};

class File
{
private:
    std::unique_ptr<Object> object;
    std::shared_ptr<File> self;

    File()
    {}

public:
    void setObjectPosition(std::uint64_t newPosition);

    static std::shared_ptr<File> create();
};

void Object::modify()
{
    std::uint64_t newPosition = …;

    file.lock()->setObjectPosition(newPosition);
}

void File::setObjectPosition(std::uint64_t objectPosition)
{
    // store objectPosition
}

std::shared_ptr<File> File::create()
{
    auto file = std::shared_ptr<File>(new File());

    file->self = file;
    file->object = std::unique_ptr<Object>(new Object(file));

    return file;
}

Is this this the right approach to do this? Or is there a better/more idiomatic solution? I feel like I'm using *_ptr too much, but I can't think of anything better.

share|improve this question
1  
I expect lifespan of the object is never longer then a file. I believe references would work just fine... –  Eugene Sep 12 '13 at 18:41
    
In your particular example object and file are really tightly coupled (1:1, dependency on concrete class). You might as well have a single combined object. –  Eugene Sep 12 '13 at 18:43
    
@Eugene Yeah, but if I have a bug in my code, I prefer to derefence a null pointer and crash the application, rather than silently overwriting someone else's memory. –  svick Sep 12 '13 at 18:55
    
@Eugene That's just an example, in my real code, there are several objects. –  svick Sep 12 '13 at 18:56
add comment

2 Answers

If all you need is a non-owning pointer to an object, just use a raw pointer. Here's a smaller example:

template <typename T> struct binary_tree_node
{
    std::unique_ptr<binary_tree_node> left_child_;
    std::unique_ptr<binary_tree_node> right_child_;
    binary_tree_node* parent_ = nullptr;
    T data_; 
};

shared_ptr and unique_ptr imply some kind of ownership, which child->parent does not have, since the parent owns the child. Therefore a raw pointer is totally okay in this situation(just don't delete it!)

share|improve this answer
    
Well, that's why I have weak_ptr from child to parent, not shared or unique. –  svick Sep 12 '13 at 23:36
    
But weak_ptr is for breaking shared_ptr cycles. If you don't use shared_ptr, you shouldn't use weak_ptr. From your question I gathered that you started using shared_ptr because you decided to store a weak_ptr to the parent. Using a raw pointer avoids this requirement. –  bstamour Sep 13 '13 at 12:09
add comment

In these cases I think it's better to use a naked pointer from the child, if you can't guarantee that the parent out lives its child just let the destructor set it to 0 to signify that a child has no parent. This way you don't need 2 phase construction, and you don't pay the overhead of weak_ptr<>::lock at every turn.

You should probably also take a look at boost::enable_shared_from_this<>, in fact with the approach you're using now with File keeping an explicit reference to itself you defeat the purpose of using it in the first place.

Furthermore it's usually a good idea to use boost::make_shared<>, as it both uses fewer allocations ( saving you memory & cycles ), and just have better cache behavior all around ( since you usually poke the ref count when you poke the object ).

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.