> 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?
>
> Ben Mahler wrote:
> 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().
I did a research on how to provide custom interval (not trivial, but doable).
This is the only option that allows us to do the following:
foreach (const Interval<T>& interval, intervalSet) { ... }
We can do whatever we want in class Interval (e.g. as you suggested). However,
when creating intervals, we need to do the following (by specifying the type):
intervalSet += Interval<int>::open(2).closed(4); // Notice that we need to
specify '<int>' here.
Or, we can create another class called "IntervalBuilder":
intervalSet += IntervalBuilder::open(3).closed(4);
Which one do you guys prefer?
- Jie
-----------------------------------------------------------
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
>
>
