> 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) {
> ..
> }
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?
- 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
>
>
