> On Feb. 14, 2014, 5:54 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/interval.hpp, lines 143-146 > > <https://reviews.apache.org/r/18093/diff/2/?file=484865#file484865line143> > > > > I'm wondering about the use-cases here to see if the current iterator > > semantics are the easiest to work with. > > > > Do you see most people using this as a plain 'set' (with a compact > > representation) or mostly as a 'set of intervals', or both? > > > > For the first use case, I can imagine calling this 'IntervalSet', but > > exposing the same 'Set' like interface as, say, hashset. > > > > For the second use case, I can imagine calling this 'Intervals', and > > exposing the current interface. > > > > For example, what should the default iterator behavior be? > > > > // Case 1: > > foreach (const T& t, set) { > > ... > > } > > > > // Case 2: > > foreach (const Interval& i, intervals) { > > ... > > } > > > > Just something to think about. It seems like you will be using this > > mostly as a 'Set'. > > Jie Yu wrote: > There is a note in the doc: > > "So we advice you to choose element iteration over interval containers > judiciously. Do not use element iteration by default or habitual. Always try > to achieve results using member functions, global functions or operators > (preferably inplace versions) or iteration over segments first." > > I guess the reason is because doing iteration on elements are somewhat > dangerous because you might have a very large interval (e.g. imaging that you > insert an interval [1,100000000]). Therefore, I think maybe we should stick > to the original boost semantics and make element iteration explicit. > > Also, changing the default behavior of begin()/end() is a bit tricky > since you cannot simply extend from boost interval_set anymore :( > > Regarding the Intervals idea, I liked that! My concern is: we haven't > seen use cases currently. Maybe we should delay that until we see some actual > use cases which then can help us design a good API. What do you think? > > Ben Mahler wrote: > Sounds great, given your comment can you also add interval iteration in > the tests? I'm curious what that looks like. > > // Would be nice to be able to do something like this for iteration. > foreach (const Interval& interval, intervalSet) { > .. > } > > Jie Yu wrote: > You have a good point. I haven't thought about the interval iteration > enough. To do the interval iteration now, you'll have to do the following: > > foreach (const discrete_interval<T>& interval, intervalSet) { > T left = interval.lower(); > T right = interval.upper(); > > if (interval.bounds() == interval_bounds::right_open()) { > > ... > } else if (interval.bounds() == interval_bounds::closed()) { > ... > } > ... > } > > Seems that we need to abstract Interval as well, which might need to > create a customized interval type. > > http://www.boost.org/doc/libs/1_53_0/libs/icl/doc/html/boost_icl/examples/custom_interval.html > > Any suggestion?
Hm, is it possible to have Interval inherit from discrete_interval, but provide simpler accessors for upper() and lower()? What I would find most intuitive is an inclusive (closed) lower() and exclusive (open) upper(), since this is how things like begin() and end() work. With these, the caller does not need to know about the bounds(). - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18093/#review34507 ----------------------------------------------------------- On Feb. 14, 2014, 5:38 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18093/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2014, 5:38 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-993 > https://issues.apache.org/jira/browse/MESOS-993 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am 51b118c > 3rdparty/libprocess/3rdparty/stout/Makefile.am 5d5a760 > 3rdparty/libprocess/3rdparty/stout/include/stout/interval.hpp PRE-CREATION > 3rdparty/libprocess/3rdparty/stout/tests/interval_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/18093/diff/ > > > Testing > ------- > > make check > > repeat 1000 times for the IntervalTest > > > Thanks, > > Jie Yu > >
