Create shared_ptr from reference

Question

I'm relativly new to C++ and this seems like a noob question but I wasn't able to solve it with other resources on the internet.

I'm trying to create a shared_ptr from a reference. I have the following Book class:

#include <memory>
#include "Author.hpp"

class Book
{
   public:
      void setAuthor(const Author& t_author);

   private:
      std::shared_ptr<Author> m_author;
}

And this is my Author class:

#include <memory>
class Book;

class Author
{
   public:
      void addBook(const Book& t_book);

   private:
      std::vector<std::weak_ptr<Book>> m_books;
}

I tried to implement the Book::setAuthor method like so:

void Book::setAuthor(const Author& t_author)
{
   m_author = std::shared_ptr<Author>(&t_author);
}

But if I try to compile this I get:

Invalide conversion from const Author* to Author*

Invalide conversion from sizeof to const Author

Can you please tell me what is wrong with my code? I also tried the same with the weak_ptr but this does not work either.

I think you should use `std::vector` and `std::vector`. No pointer, no `std::shared_ptr`, no `std::weak_ptr`. Manage the relationship between books and authors in a **relational database**; that's what they are for. Treat the `std::vector` and `std::vector` objects as temporarily needed results of database queries. — Christian Hackl, Feb 08 '17 at 19:47
@ChristianHackl You are right and I also want this but I would like to use a [ORM library](http://www.codesynthesis.com/products/odb/) and there I have to use `vectors` of `shared_ptr` to represent [one to many](http://www.codesynthesis.com/products/odb/doc/manual.xhtml#6.2.2) relationships. Or is there another way for doing this? — Cilenco, Feb 08 '17 at 22:40
@Cilencio: Well, why use a library if it forces you to use a flawed design? ORM is not a goal in itself. — Christian Hackl, Feb 09 '17 at 06:14
@ChristianHackl Okay thanks I will think about it. But am I right that I have to use `std::vector` then because the classes (headers) dependent on each other and I have to use forward declaration for one class (here `Book`)? — Cilenco, Feb 09 '17 at 15:13
@Cilencio: Yes, if `Book` contains a `std::vector` and one `Author` contains a `std::vector`, then you'd have infinite recursion, so it's not possible. I think an important question has not been asked so far: What operations will you perform on this data structure? If, e.g., the goal is to display a list of books with a list of authors for each item, then it's not necessary for `Author` to contain a `std::vector`. — Christian Hackl, Feb 10 '17 at 06:15
@Cilcenio: P.S.: `std::vector` will work but may be harder to get right than the original `std::weak_ptr` solution. It depends. The big underlying architectural problem you are experiencing here is called the *object-relational impedance mismatch*. Books and authors who wrote them are a classical example for relational data, yet you try to fit it into an object-oriented, or parent-child, data structure. You will find a lot of reading material on this topic on the internet. Make sure to search also in softwareengineering.stackexchange.com. — Christian Hackl, Feb 10 '17 at 06:22

WhiZTiM · Accepted Answer · 2018-02-16T09:53:28.040

29

Though, your error stems from the fact that the std::shared_ptr<Author> constructor in use expects Author*, but the expression &t_author results to an object of type const Author*

Another wrong thing:

void Book::setAuthor(const Author& t_author)
{
   m_author = std::shared_ptr<Author>(&t_author);
}

Imagine calling book.setAuthor(Author("Herb Sutter"));, you will have a dangling pointer because t_author will cease to exist after that function completes.

You need to copy or move the object into your std::shared_ptr instance. Use std::make_shared<T> to create your std::shared_ptr<T> objects whenever possible.

void Book::setAuthor(const Author& t_author)
{
   m_author = std::make_shared<Author>(t_author);
}

Better still:

void Book::setAuthor(Author t_author)
{
   m_author = std::make_shared<Author>(std::move(t_author));
}

edited Feb 16 '18 at 09:53

answered Feb 08 '17 at 19:24

WhiZTiM

21,207
4
43
68

Thank you for the answer that helped me a lot. Can you please have a look on my comment below the question? What do you think would be the best design pattern for the one to many approach? Or shouldn't I use such an ORM library? – Cilenco Feb 08 '17 at 22:55
1

@Cilenco: *"Should I use an ORM library?"* is a very broad and opinion-based question! :) I think the only answer we can give you is to choose the right tool for your software's (or your employer's) requirements, never to use a tool just so you can use a tool, and keep in mind that every solution has benefits and liabilities. Who knows, even C++ itself may be a bad choice for this specific problem, but we cannot say without understanding the problem. – Christian Hackl Feb 09 '17 at 06:46
4

Does not the latter case cause the copying of `Author` object being passed? – St.Antario Feb 12 '19 at 14:13

Slava · Answer 2 · 2017-02-08T19:30:39.947

9

If you want to make a copy use std::make_shared:

void Book::setAuthor(const Author& t_author)
{
   m_author = std::make_shared<Author>(t_author);
}

but this is a wrong design, if you expect to keep ownership of passed objects you should pass std::shared_ptr to your function instead of const reference:

void Book::setAuthor( std::shared_ptr<Author> t_author)
{
   m_author = std::move( t_author );
}

edited Feb 08 '17 at 19:30

answered Feb 08 '17 at 19:26

Slava

43,454
1
47
90

So with my design I wouldn't be the owner anymore of the reference? Can you please explain this in more detail? Do you may have also ideas for a good design pattern with the odb (orm) library (see comments under the question) – Cilenco Feb 08 '17 at 22:59
In your design you pass object by const reference, so you can call const methods of it or make a copy. In your class you use `std::shared_ptr` which suggests shared ownership. Something wrong here you should either not use `std::shared_ptr` or pass objects that already owned by `std::shared_ptr` for your class to be able to share. – Slava Feb 08 '17 at 23:02
And there is no such thing as owner of the reference. If you pass object by reference you assume that function you pass it to cannot take or share ownership of it. – Slava Feb 08 '17 at 23:04

score 3 · Answer 3 · answered Feb 08 '17 at 19:27

This is likely undefined behavior. shared_ptr indicates ownership of the object it points to. In just about every conceivable scenario, t_author refers to an existing Author that is owned by something else. There will almost surely be two locations that try to destroy the instance.

If you must create a shared_ptr to an existing instance, you can look into using enable_shared_from_this, but this only works if t_author was created with std::make_shared. And if this is the case, you might as well change your function to accept the shared_ptr directly. Alternatively, you can create a shared_ptr with a custom deleter that does nothing. But at that point, there is nothing to gain from using shared_ptr, except perhaps compatibility with some interface.

Create shared_ptr from reference

3 Answers3

Linked