Task #3158
closedRefactor Interest::getLink to save parsed Link object
100%
Description
Currently, Interest::getLink
attempts to create a new instance of Link object.
As part of #3155 and #3034 it was decided that getLink should remember the parsed Link object within internal shared_ptr<Link>
variable (cannot use unique_ptr
as it has deleted copy constructor). This also implies that the signature of getLink method needs to be changed to
const Link&
getLink() const;
Updated by Junxiao Shi about 9 years ago
unique_ptr
should be used. When the Interest
is copied, simply don't copy the unique_ptr
, and the second Interest
instance may re-parse the Link object if necessary.
It's harmful to use shared_ptr
because if someone decides to const_cast
and modify the Link returned by one Interest::getLink
, it would affect other instances.
Updated by Alex Afanasyev about 9 years ago
I disagree. If you consider the case of violation of const access restriction, then you already have a bunch of issues with anything that return const&.
Updated by Alex Afanasyev about 9 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
Updated by Davide Pesavento about 9 years ago
If you return a const Link&
, the returned reference could outlive the referenced Link object, e.g.:
const Link& link = interest.getLink();
interest.unsetLink(); // or interest.setLink(anotherLink);
link.getDelegations(); // boom!
why not returning the shared_ptr
? (this is the whole point of shared pointers... their purpose is to share ownership of an object)
Updated by Alex Afanasyev about 9 years ago
One can always call .shared_from_this()
if he/she needs to retain the instance. The only reason I'm against returning shared_ptr
as it clutters the interface.
The issue Davide pointed is kind of possible, but I really don't want to care about this and open this can of worms (you can say the same about wireEncode(), about getName(), and about many other getters in many different classes). If one write such a statement, then his app should crash.
Updated by Alex Afanasyev about 9 years ago
- Status changed from Code review to Closed