> On Dec. 15, 2014, 1:29 p.m., Ben Mahler wrote: > > include/mesos/resources.hpp, lines 211-214 > > <https://reviews.apache.org/r/29018/diff/1/?file=791218#file791218line211> > > > > To keep consistent with the rest of the code base, couldn't you have > > done the following? > > > > ``` > > template <typename T> > > void add(const T& t) > > { > > transformations.push_back(new T(t)); > > } > > ``` > > > > With unique_ptr, we're requiring that ownership be transferred if the > > caller is holding the transformation, which seems a bit odd for our library > > to impose on the caller: > > > > ``` > > // The caller cannot keep ownership of t. > > std::unique_ptr<Transformation> t(new DiskTransformation(...)); > > composite.add(std::move(t)); > > > > // Ok. > > composite.add(std::unique_ptr<Transformation>(new > > DiskTransformation(...)); > > ``` > > > > Could we keep the const argument semantics for now for consistency? > > Then the caller doesn't have to use 'new' as well. Thoughts?
That's exactly the point of std::unique_ptr - a library is finally able to inform the caller that they're taking responsibility for the object and the memory associated with it. I don't see that as odd. Can you describe a use case where a caller might want to retain a reference to the transformation? - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29018/#review65121 ----------------------------------------------------------- On Dec. 12, 2014, 9:57 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29018/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2014, 9:57 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2030 > https://issues.apache.org/jira/browse/MESOS-2030 > > > Repository: mesos-git > > > Description > ------- > > Added abstraction for resources transformation. Split from > https://reviews.apache.org/r/28720/. > > > Diffs > ----- > > include/mesos/resources.hpp 296553af93ec8457dad04ac018f03f36df654fdc > src/common/resources.cpp 9bf7ae9148d6db2829cc2866ac048fe018ae2c92 > > Diff: https://reviews.apache.org/r/29018/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
