Issue 11252 - In operator requested for std.range.iota

[dukc]

I decided against implementing it for floating pointed iotas because comparing floating points is so unreliable.

[dlang-bot]

Thanks for your pull request, @dukc! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✗	11252	"in" operator for std.range.iota

[JackStouffer]

please keep the space

[dukc]

Too small a detail to be worth cleaning imo, but as you wish.

[wilzbach]

It churns the git blame and if we don't pay attention to avoid unrelated changes, git blame becomes useless. Also it makes things a lot harder for people who skim over the diffs.

[JackStouffer]

I still don't see much utility in this TBH. The proposed usage in the bug report is way slower than saving to a temporary and doing two comparisons in an if statement.

[dukc]

Check what was said later in the bug report... It does not do a linear search, this is a constant-time algorithm.

[quickfur]

I'm on the fence about this one. Using an operator overload on iota seems like a very roundabout way of doing what the bug report is asking for. Why can't this just be a standalone range (in the numerical sense, not in the sense of input range) comparison function?

bool liesBetween(string boundsType="[)", T)(T val, T lower, T upper)
{
    static if (boundsType="[)")
        return lower <= val && val < upper;
    else
        ... // other boundsType cases, it's just 4 of them
}

assert(5.liesBetween(3, 6));

Note that the original bug report isn't even asking for testing membership in iota with non-unit step, so implementing an operator overload for iota really seems like overkill. It's not as though we have a general membership test for generic ranges, either, so it also feels somewhat out-of-place.

[quickfur]

Also, if iota is applied to a custom user type that supports ++ but not arithmetic in general, then it's not possible to implement operator in in an efficient way, in which case I would be against adding it to Phobos. But if we exclude those instances of iota, then it seems incongruous (sometimes iota supports in, sometimes it doesn't).

I think a standalone comparison function is still better -- no need to instantiate iota just to check if something lies between lower/upper bounds, and no need to worry about instantiations of iota where bounds checking cannot be implemented efficiently.

[dukc]

What comes to custom types only incrementing, I think I must add a template constraint forbidding usage with non-integral types. That constraint is already there for indexing, so nothing special there.

But I agree that may be a bit inconsistent way with the other library to solve this problem. On the other hand, somebody might already have an iota that needs to be checked, and forcing to retype it's parameters is a bit suboptimal. Only bit, trough.

Isn't this question somewhat like c++ iterator pairs vs ranges? In that should one use two arguments to describe bounds for an area, or one where the bounds have been premade?

I'll add the constraint. If you are positive it should be a three-argument between function instead of iota, close this. On the other hand, I could also change the in operation to a regular member function.

EDIT: opIndex does not have any contraints, so none needed for opBinary!"in" either. In fact, the whole template requires the type to fulfill IsIntegral or IsPointer, and at least according to docs that means only built-in types. So no worries about customs.

[JackStouffer]

@wilzbach This doesn't add a new symbol, why do you want Andrei's review for this?

[dukc]

An operator is a symbol, or at least equivalent to a symbol. Or do you mean module-level symbols?

[wilzbach]

@wilzbach This doesn't add a new symbol, why do you want Andrei's review for this?

Because this seems to be like an important precedent (and is about adding a new symbol): "should we add in operator support to ranges or add a new symbol?"

[andralex]

This is useful. In fact it would be nice if iota supported some of the methods in SortedRange, too.

Please add documentation and documentation unittests.

[JackStouffer]

Well, since Andrei has approved this concept, there are two things that need to be fixed:

• Docs
• Add test for user defined types e.g. BigInt

[dukc]

Wow, I did not even notice that iota for user-defined types before now. I agree it should support in operator if the type was fully arithmetic, but because it does not support random access anyway it is matter of a separate pr. So no need for custom-type unittests either.

I'll do the documentation, trough.

[PetarKirov]

Add test for user defined types

That's not as easy as it sounds (you can read in some bugzilla issue why), so we should leave that for another PR.

@dukc mind the docs and this should be good to go.

[dukc]

I won't probably do it this nor the next week because I have other stuff to do but then, I will.

[andralex]

One more thing: iota should support contains similar to SortedRange. The backend of in and contains is the same.

[dukc]

Documentation done.
Edit: Also added contains at the same time.