maybe add a comment saying that size() is only used for testing. - Jie
On Fri, Feb 5, 2016 at 6:07 PM, Michael Park <mp...@apache.org> wrote: > I also spoke to Jie about this issue, and he agreed that it'd be better to > expose the `size` rather than people trying to compute it manually. > > On 5 February 2016 at 18:04, Michael Park <mp...@apache.org> wrote: > >> >> On 5 February 2016 at 17:15, Benjamin Mahler <bmah...@apache.org> wrote: >> >>> Didn't we pull this out in the past? I seem to remember it being too >>> brittle to have code looking at 'size' since it depends on our changing >>> concept of when Resource objects can be merged. >>> >> >> Yep, I brought up the fact that we intentionally removed `size()` before >> here: https://reviews.apache.org/r/42751/#comment178972. Neil brings up >> the point that as long as we support iteration over the `Resources`, people >> can compute the size themselves anyway and therefore is part of the public >> API. Another situation was when we wanted to test that resources were >> merged or not merged, there's no good way for us to test either of the >> conditions. >> >> >>> On Fri, Feb 5, 2016 at 3:16 PM, <mp...@apache.org> wrote: >>> >>>> Added `Resources::size()`. >>>> >>>> Review: https://reviews.apache.org/r/43239/ >>>> >>>> >>>> Project: http://git-wip-us.apache.org/repos/asf/mesos/repo >>>> Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d9d966d9 >>>> Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d9d966d9 >>>> Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d9d966d9 >>>> >>>> Branch: refs/heads/master >>>> Commit: d9d966d9e636fd4bee8b902742eaa9cf6dd1b342 >>>> Parents: 2a65da0 >>>> Author: Neil Conway <neil.con...@gmail.com> >>>> Authored: Fri Feb 5 14:11:33 2016 -0800 >>>> Committer: Michael Park <mp...@apache.org> >>>> Committed: Fri Feb 5 15:04:09 2016 -0800 >>>> >>>> ---------------------------------------------------------------------- >>>> include/mesos/resources.hpp | 2 ++ >>>> include/mesos/v1/resources.hpp | 2 ++ >>>> 2 files changed, 4 insertions(+) >>>> ---------------------------------------------------------------------- >>>> >>>> >>>> >>>> http://git-wip-us.apache.org/repos/asf/mesos/blob/d9d966d9/include/mesos/resources.hpp >>>> ---------------------------------------------------------------------- >>>> diff --git a/include/mesos/resources.hpp b/include/mesos/resources.hpp >>>> index 6bfac2e..fe8a574 100644 >>>> --- a/include/mesos/resources.hpp >>>> +++ b/include/mesos/resources.hpp >>>> @@ -209,6 +209,8 @@ public: >>>> >>>> bool empty() const { return resources.size() == 0; } >>>> >>>> + size_t size() const { return resources.size(); } >>>> + >>>> // Checks if this Resources is a superset of the given Resources. >>>> bool contains(const Resources& that) const; >>>> >>>> >>>> >>>> http://git-wip-us.apache.org/repos/asf/mesos/blob/d9d966d9/include/mesos/v1/resources.hpp >>>> ---------------------------------------------------------------------- >>>> diff --git a/include/mesos/v1/resources.hpp >>>> b/include/mesos/v1/resources.hpp >>>> index 5a88c07..c27927e 100644 >>>> --- a/include/mesos/v1/resources.hpp >>>> +++ b/include/mesos/v1/resources.hpp >>>> @@ -209,6 +209,8 @@ public: >>>> >>>> bool empty() const { return resources.size() == 0; } >>>> >>>> + size_t size() const { return resources.size(); } >>>> + >>>> // Checks if this Resources is a superset of the given Resources. >>>> bool contains(const Resources& that) const; >>>> >>>> >>>> >>> >> >
