> On Nov. 25, 2015, 5:01 a.m., Michael Park wrote: > > src/master/http.cpp, lines 541-551 > > <https://reviews.apache.org/r/40247/diff/6/?file=1137470#file1137470line541> > > > > As far as its implementation, let's do: > > > > ```cpp > > static Resources removeDisks(Resources resources) > > { > > foreach (Resource& resource, resources) { > > resource.clear_disk(); > > } > > return resources; > > } > > ``` > > Neil Conway wrote: > Is this really better? You C++ guys are so tricky with your value > semantics :) > > To me, the version in the review makes it more obvious that we are > copying the input value, mutating the copy, and then returning the copy. > Whereas in your version, at first glance the reader might think the function > modifies its input value in place. > > What do you think? Happy to make the change if you think it is an > improvement. > > Michael Park wrote: > Well, I suppose "better" is subjective. It's more efficient, > but I didn't think (and don't think) that this is less readable. But that > may be just me. > > Also, the code that fits your description would be: > > ```cpp > static Resources removeDisks(const Resources& resources) > { > Resources result = resources; // copying the input value. > > // mutating the copy. > foreach (Resource& resource, result) { > resource.clear_disk(); > } > > return result; // returning the copy. > } > ``` > > This version has half as many copies as the one in the review, and (I > think) is also just as readable. > > So you have 3 options, I'll leave it upto you :)
Cool -- I went with the last version (copy input and then mutate the copy in-place). Thanks for the advice! :) - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40247/#review107913 ----------------------------------------------------------- On Nov. 26, 2015, 9:26 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40247/ > ----------------------------------------------------------- > > (Updated Nov. 26, 2015, 9:26 p.m.) > > > Review request for mesos, Greg Mann and Michael Park. > > > Bugs: MESOS-2455 > https://issues.apache.org/jira/browse/MESOS-2455 > > > Repository: mesos > > > Description > ------- > > Added HTTP endpoints for creating and destroying persistent volumes. > > > Diffs > ----- > > docs/persistent-volume.md 0c9e0e93c3ae8e00f841303e8c2b26b36a775eac > docs/reservation.md e297921e709838a6780c58a12637b261fa7f18cb > src/Makefile.am de68e24fb2ad4c6e4175fbf8658b23bc6434a356 > src/master/http.cpp cffd20b2557b84b415940ab9af8d374c71b6627b > src/master/master.hpp 0bb315a16801de9e7014ca0a83c5152faa75eb38 > src/master/master.cpp 92380952277ae3fe0b535718b6b1b8732e960745 > src/tests/mesos.hpp eabbf44626b0e14d93febb55b1b22c9ad236daa1 > src/tests/persistent_volume_endpoints_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/40247/diff/ > > > Testing > ------- > > (1) make check, including newly added tests > > (2) Manually created/removed persistent volumes via HTTP endpoints + curl. > > (3) Previewed docs in Github gist. > > > Thanks, > > Neil Conway > >
