SE-0225: Adding isEven, isOdd, isMultiple to BinaryInteger


(John McCall) #1

The review of SE-0225: Adding isEven , isOdd , isMultiple to BinaryInteger begins now and runs through Sunday, August 26th, 2018.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager via email or direct message on the forums. If you send me email, please put "SE-0225" somewhere in the subject line.

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?

  • Is the problem being addressed significant enough to warrant a change to Swift?

  • Does this proposal fit well with the feel and direction of Swift?

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thank you for contributing to Swift!

John McCall
Review Manager


[Accepted with Modifications] SE-0225: Adding isMultiple to BinaryInteger
`isNotEmpty` on Array
(Chéyo Jiménez) #2
  • What is your evaluation of the proposal?

+1 for multiple and isEven, but I do not believe we need isOdd.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Sure.

  • Does this proposal fit well with the feel and direction of Swift?

Yes

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read implementatio.


(^) #3

Oppose. not to be a negative nancy but i think Ben Cohen’s “too much sugar not enough protein” quote applies to this proposal. A lot of the justifications given in the proposal don’t seem to hold water, examples:

Readability: This proposal significantly improves readability, as expressions read like straightforward English sentences. There is no need to mentally parse and understand non-obvious operator precedence rules ( % has higher precedence than == ).

since when was %’s and ==’s relative precedence non-obvious? no one seems to think

if value / 2 == 0

has confusing precedence after all. >> and & are examples of operators with confusing precedence. the ternary ? : is an operator with confusing precedence. an arithmetic operator vs a comparison operator is not confusing precedence…

The isEven and isOdd properties are also fewer characters wide than the remainder approach (maximum 7 characters for .isEven vs 9 for % 2 == 0 ) which saves horizontal space while being clearer in intent.

2 character difference,,, really just leave out the spaces before the colon in the type annotation and you’ll save just as much…

This functionality will also eliminate the need to use the remainder operator or bitwise AND when querying the divisibility of an integer.

we love circular reasoning

Correctness: It isn't uncommon to see tests for oddness written as value % 2 == 1 in Swift, but this is incorrect for negative odd values. The semantics of the % operator vary between programming languages, such as Ruby and Python, which can be surprising.

The % operator will also trap when the righthand side is zero. The proposed solution does not.

99% of the time we test for divisibility, the number on the right hand side is a literal constant. it’s unlikely someone will intentionally write value % 0, after all you wouldn’t write value / 0. testing against negative numbers is rare and usually arises in special domain-specific cases and you’re probably already going to be handling those separately anyways.

Discoverability: IDEs will be able to suggest isEven , isOdd , and isMultiple as part of autocomplete on integer types which will aid discoverability. It will also be familiar to users coming from languages that also support functionality similar to isEven and isOdd .

Testing the parity of integers is also relatively common in sample code and educational usage. In this context, it’s usually not appropriate for an author to introduce this functionality (unless they are teaching extensions!) in order to avoid distracting from the main task at hand (e.g. filter, map, etc). It may also be the same situation for authoring test code: it'd be used if it existed but it's not worth the overhead of defining it manually.

in general, discoverability and teachability are rationales we come up with when we can’t think of anything else to justify a feature

no. sorry :(

no

it’s one of those sugar things that can be nice to have but i think just doesn’t clear the bar for inclusion. i’m open (but not crazy in favor) of isMultiple(of:) by itself. i don’t think isEven or isOdd carry their weight in the standard library at all.

i followed the pitch thread and read the proposal through.

i want to make clear this review isn’t in any way a criticism of the proposal authors, I just don’t think the reasons given justify adding these parity properties to the standard library forever.


#4
  • What is your evaluation of the proposal?

+1, with bonus for making the topic unexpectedly entertaining. Special thanks to @Dany_St-Amant for inventing isMultiple(of:).

  • Is the problem being addressed significant enough to warrant a change to Swift?

I'm sold to the "Commonality", "Correctness", "Readability", and "Trivially composable" arguments of the proposal.

  • Does this proposal fit well with the feel and direction of Swift?

Yes

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Just as good.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed the pitch, read the proposal.


(Jeremy David Giesbrecht) #5

Definitely useful. I have been using an extension with very similar methods since Swift first came out.

On the other hand, that very extension was trivial to implement. If that makes it not significant enough to embed in the Standard Library, it would not bother me.

I read the entire proposal, but only the beginning of the the pitch thread.

One question though:

The proposal implements isEven like this:

@_transparent
public var isEven: Bool { return _lowWord % 2 == 0 }

@_transparent
public var isOdd: Bool { return !isEven }

Wouldn’t the following be more efficient, or does compiler optimization eliminate the difference?:

@_transparent
public var isEven: Bool { return !isOdd }

@_inlineable
public var isOdd: Bool { return self & 1 == 1 }

(Steve Canon) #6

To the extent that these are different from the compiler's perspective, that's a bug that should be fixed. (Assuming you mean _lowWord & 1 in your suggestion; self & 1 is potentially significantly less efficient in the presence of user-defined integer types.)


(Morten Bek Ditlevsen) #7

+1 I think all three additions are closer to the way most people reason about code than the ‘modulo’ counter parts.

I think that sugar like this is a good fit. The clarity argument is sufficient for me. That said, I am a bit unsure about how high the bar for additions should be set. But my feeling is that a syntactic addition that brings ease of reasoning should always be welcome.

I think it very much fits with any goals about Swift being a good beginner‘s language.

I have not.

Followed the discussion, read the proposal, read arguments about ‘sugar versus protein’ and still I can only see positive effects of adding this to the language.


(Jeremy David Giesbrecht) #8

Yes, that is what I meant. I had searched for the implementation of _lowWord, to get an idea of its complexity and saw this:

The ellipsis escaped my eye, so I was very confused about why the implementation was so complicated, how it would ever work and if I really understood what it was supposed to do after all.

Now that I see the ellipsis you can all laugh at me. :laughing:


(Benjamin Mayo) #9

What's your evaluation of the proposal?

Mildly in favour of adding isEven, but against the other two proposed additions. Ignoring minor performance benefits, I think there is a nice readability and clarity win of using isEven rather than % 2 == 0 or similar constructions. I haven't done quantitive research, but instinctively checking for even-ness is something that is common enough to warrant its inclusion as a property. I also think there's a linguistic parallel that aids beginners, it's way more natural to think "is this number even?", not "is this number evenly divisible by two".

I am against adding isOdd. I don't see why here we should have a trivial inverse accessory, where there are plenty of other accessors in the standard library that do not have complementary inverses. I would be against adding 'isNotEmpty' as an alternative to !isEmpty, so it logically follows to me that isOdd should be barred for the same reason.

I don't come away from reading the proposal believing that isMultiple should be added to the standard library. My personal opinion is that isMultiple was tacked onto the original isEven/isOdd proposal as a way to make the proposal feel like it has a substantive meaning, in the face of some lingering community criticism that going through Evolution for something 'as simple as isEven' was ridiculous. I really don't agree with that argument, and don't think isMultiple should be included here for the sake of it. In isolation, I think isMultiple might make sense but it needs more justification in a separate pitch, in my opinion.

Is the problem being addressed significant enough to warrant a change to Swift?

Minor addition, minor convenience. It seems proportionate to add isEven.

Does this proposal fit well with the feel and direction of Swift

I believe so. It's still hard to work out exactly where Swift standard library should straddle the line of convenience properties methods that are relatively 'trivial'. For me, checking even-ness on an integer is roughly equivalent to splitting a string into lines. Both of these things are relatively straightforward at some level, but I think both would actually feel 'right' as part of the standard library APIs. It's easy for experienced developers, including me, to laugh at writing num.isEven versus num % 2 == 0. But I think someone new to programming, with no history of any programming language, would acclimatise and discover isEven far more naturally than they would the remainder operator and checking for a specific value.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No comment here. I feel like this should be evaluated on the design and scope goals of the Swift standard library, and whether another language API offers it or not is largely irrelevant.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Followed (ghosted) the evolution discussion and formulation of the proposal. I also read the proposal in full.


(Jacob Williams) #10

What is your evaluation of the proposal?
+1 isEven and isMultiple
-0.5 for isOdd

Is the problem being addressed significant enough to warrant a change to Swift?
Checking whether a number is a multiple of another is probably common enough to warrant built in support. I'm sure not everyone uses it every day, but it has widespread uses/needs in many kinds of Math-based programming.
isEven/isOdd are trivial to add, but apparently they're pretty easy to mess up too. I'm all for isEven and I agree with the argument that we don't generally have inverses in the stdlib which is why I'm -0.5 on isOdd. !.isEven is still clearly understood to mean isOdd. So I'm putting aside my personal desires to have isOdd and voting against its inclusion.

Does this proposal fit well with the feel and direction of Swift?
Except for isOdd. See above.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
Kinda hard to get any easier than myNum.isEven. Pretty similar to other languages and it should be easy to find.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I've participated in the original pitch/proposal, read the proposal through, and did some outside studying/experimenting.


(Boris Triebel) #11

+1
yes
yes
n/a
Followed the pitch, read the proposal.


(Xiaodi Wu) #12

I am, in the end, rather neutral to the specific proposed additions but leaning slightly against their inclusion, for the same reason as @taylorswift cites: "too much sugar, not enough protein."

However, I believe the justifications advanced in the text are very much inconsistent with the direction of Swift. If this proposal were to be accepted, then those justifications will be cited as precedent for other proposals. Therefore, whatever the fate of the proposed additions themselves, I believe the proposal text itself should be revised or rejected.

Specifically:

There is no need to mentally parse and understand non-obvious operator precedence rules ( % has higher precedence than == ).

While there are non-obvious operator precedence rules, the precedence of == in relation to basic arithmetic operators is not one of them. It is not the purpose of Swift's methods to allow the user to avoid even the most basic use of common operators. This claim should form no part of any justification for adding new APIs.

The isEven and isOdd properties are also fewer characters wide than the remainder approach (maximum 7 characters for .isEven vs 9 for % 2 == 0 ) which saves horizontal space while being clearer in intent.

While Swift does, in general, favor concise APIs, in no case is an API that's spelled using 7 characters more readable than one spelled using 9 characters because there are two fewer characters (and whitespace at that!). This is a non-goal of Swift API design, and no proposal now or in the future should cite such a point as justification for a new API (or even renaming an existing one).

Discoverability: IDEs will be able to suggest isEven , isOdd , and isMultiple as part of autocomplete on integer types which will aid discoverability. It will also be familiar to users coming from languages that also support functionality similar to isEven and isOdd .

The text above is the entirety of the section on why the proposed APIs "aid discoverability." It is axiomatically true that any new member will be found in the autocomplete list; if this argument were sufficient to fulfill the criterion of discoverability, then any addition of a method would fulfill that criterion.

That should not stand: At a minimum, adding methods to a type also decreases the discoverability of all other methods of that type by virtue of the fact that the list of methods grows longer. Integer types already have a very complicated API. This is a trade-off that is not at all represented in this discussion. Moreover, removing one of the most common uses for % decreases the user's opportunities to encounter (and eventually master) a common operator, decreasing the discoverability of that operator.

Trivially composable: It would be relatively easy to reproduce the proposed functionality in user code but there would be benefits to having a standard implementation.

Again, this text is eviscerating the meaning of the criterion. These methods are, for the most part, trivially composable. (Which is fine--a proposed addition could still pass the bar for inclusion based on fulfilling multiple other criteria.) However, if this text were accepted as justification for adding APIs, then almost any trivially composable implementation would pass the bar, as it is axiomatically true that "there would be benefits to having a standard implementation"--that benefit being, namely, that the method is so trivially composable that "it's not worth...defining it manually" if there weren't a standard implementation, as the authors write in the text.

Testing the parity of integers is also relatively common in sample code and educational usage. In this context, it’s usually not appropriate for an author to introduce this functionality (unless they are teaching extensions!) in order to avoid distracting from the main task at hand (e.g. filter, map, etc). It may also be the same situation for authoring test code: it'd be used if it existed but it's not worth the overhead of defining it manually.

See above. The authors write that, in many situations, the function is so trivially composable that it'd not even be worth having if the standard library didn't vend it (so that even an import statement isn't necessary, lest that be too much overhead also).


Is the problem addressed significant enough to warrant a proposal? I do worry that the question of testing for oddness correctly is sufficient to merit an API addition, and for that reason I wouldn't reject the proposed APIs out of hand. That said, during the discussion phase, it was pointed out that there were no examples where the "incorrect" way of testing for oddness in the wild ever caused incorrect behavior, as users only ever wrote that when the numbers they were testing could only ever be positive.

As the authors write, the proposal doesn't fit with the direction of Swift in that it adds a pair of methods that are easy and obvious negations of each other. It's isOdd that has any chance of promoting increased correctness, and it would be (ahem) odd to have only isOdd. (For some reason, intuitively, it seems much less strange to have only isEven.)

I have used other languages with this feature, especially Python; however, I have never felt the need to reach for these APIs instead of the much more common idiom of using % even in those languages.

I have followed the previous discussions and studied this topic in-depth.


#13

I think the most genuinely useful operation in this space is a better remainder function. In particular, when I need to find the remainder when x is divided by y, I always want the result to have the same sign as y, and a magnitude strictly less than that of y.

For the purposes of this post I will refer to this operation by the deliberately unappealing name “goodRemainder”.

Now, when y is positive, that means x.goodRemainder(y) is either positive or zero, and it is in the range 0..<y. This lets us quickly and easily compose everything that is proposed in the current pitch.

Notably, goodRemainder avoids the correctness trap inherent in “x%2 == 1”, because “x.goodRemainder(2) == 1” works properly.

Furthermore, goodRemainder can be used for a wide variety of problems that involve partioning, binning, and/or wrapping-around. For a given divisor y, there are exactly abs(y) possible outputs from x.goodRemainder(y).

Alternatively, if people think it is better to have positiveRemainder instead, which always gives results in 0..<abs(y), that would be acceptable as well.

In any case, the existing remainder functionality, which preserves the sign of x and can thus produce values in 1-abs(y) ..< abs(y), is error-prone and generally not what is needed.

• • •

So… if we can get a better remainder function, then the current proposal is superfluous.


(Xiaodi Wu) #14

I believe that in most languages that observe the distinction, Swift's current operation is known as "remainder" and what you describe is known as "modulo." I think it would fit in just fine to expand Swift's integers to include a method modulo(_:), but that's kind of a different proposal from this one.


(Jeremy David Giesbrecht) #15

I actually find myself in agreement here, especially regarding the “non‐obvious” precedence rules, 7 vs 9 characters, and IDE suggestions.

Thinking back on the extensions I mentioned, they mostly just came as bonus sugar created alongside the main purpose: at the time I needed an API alternative based on the Euclidean modulo—which, as already noted, is a completely separate beast.


(Nobody1707) #16

What is your evaluation of the proposal?
+1

Is the problem being addressed significant enough to warrant a change to Swift?
These are very common operations that are inevitably going to end up being written by everyone eventually, so while they are somewhat trivial to implement, I think they're worth including. Especially since the _lowWord optimization is not at all obvious. I'd like to point out that isOdd is the only one of the pair I've seen done wrong on anything like a regular basis, so should be accepted if any of these are. Also, both isOdd and isEven read as positive assertions, so I don't think they share the same problems that other negated boolean properties do.

Does this proposal fit well with the feel and direction of Swift?
Yes.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
Yes, it's essentially the same feature set as other languages, but isMultiple(of:) is new.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
I've been following the thread closely since it started, and I've read the proposal in depth.

P.S. I don't entirely disagree with @xwu that the justifications section should be rewritten, but I would like to point out that parity is a very fundamental property of numbers, so even if the justifications weren't rewritten, it wouldn't be too hard to explain why justifications for adding these properties aren't necessarily sufficient for other easily implementable proposals.


(^) #17

i think the thing about sugar proposals is that everything is a fundamental property to someone


(Kiel Gillard) #18

I did a quick read of the proposal with a casual look at the implementation.

I like how this proposal complements the Character Properties proposal in spirit by providing API for the properties of numbers. To my mind, this makes it a good fit for the feel and direction of Swift.

I'm not too sure what @_transparent means so I'll put this out there: for performance oriented users, could there be a unexpected performance cost if some compiler optimisations are lacking? How important is consistency with other languages with regard to this for those kinds of users? How do we communicate to performance oriented users (if we even need to) that they should use value % 4 == 0 instead of value.isMultiple(of: 4) to get every bit of performance from their implementation?


(Thomas Roughton) #19

What is your evaluation of the proposal?

A very slight -1 for isEven and isOdd and a definite +1 for isMultiple(of:)

Is the problem being addressed significant enough to warrant a change to Swift?

I think the size of the change (in terms of actual code introduced) is proportional to the significance of the problem, so yes.

Does this proposal fit well with the feel and direction of Swift?

isMultiple(of:) definitely feels a Swift-like API – it's a more readable and clearer alternative to the modulo operator, and helps to avoid some subtle bugs. I think there's clear precedent in e.g. quotientAndRemainder.

isEven and particularly isOdd are less clear-cut to me. On one hand, I think that anyone who's actually using isEven or isOdd in their codebase could just define an extension in terms of isMultiple(of:); the _lowWord % 2 trick could be folded in as a special case for other <= Words.Element.max in the implementation of isMultiple(of:). On the other hand, there's not really much cost to having isEven or isOdd; I just personally can't think of many practical use cases.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I've followed some of the discussion thread and quickly read through the proposal.


#20

What is your evaluation of the proposal?

Against isOdd as adding pure inverse of function simply "pollute" the discoverability of other functions by adding "needless noise" to the auto-completion option.

Neutral on isEven and isMulitple(of:). Both are not required and can simply be expressed as value % multiple == 0; just like with Array where one can use array.count == 0 instead of the sugary array.isEmpty (does not seem to be the current exact implementation, but still a valid one)

The isMultiple(of:) was added, I think, as a generalization of isEven. Providing a generalized version is a good thing, but in this case the generalized version is not used by the implementation of the specific one (for performance optimization); loosing some of the benefit of having a generic version.

Is the problem being addressed significant enough to warrant a change to Swift?

Of the lot, only isOdd (aka !isEven) might be seen as a risk of being improperly implemented by the user, as such it may warrant the provinding isEven. The possible bad implementation being due to the version of modulo/remainder provided by % in Swift (which is not necessarily the wrong version as all three seems to be the proper one... depending on the situation). Even if there was a proposal to provide all three version of modulo/remainder, the risk of a user improperly implementing a isOdd will remain as the wrong modulo (for the task) can then be chosen.

Does this proposal fit well with the feel and direction of Swift?

The naming does fit, but might be borderline too sugary for what it provides.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Mainly follow the thread,