> On Jan 31, 2017, at 2:46 PM, Ben Cohen <[email protected]> wrote:
> 
>> 
>> On Jan 31, 2017, at 12:18 PM, Matthew Johnson <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>>> 
>>> On Jan 31, 2017, at 1:16 PM, Ben Cohen via swift-evolution 
>>> <[email protected] <mailto:[email protected]>> wrote:
>>> 
>>> I think whether enumerated() is justified as a method on Sequence is one 
>>> example of a wider question which definitely needs some discussion, which 
>>> is: where should the standard library draw the line in providing 
>>> convenience functions that can easily be composed from other functions in 
>>> the std lib? Here’s another example: 
>>> 
>>> SE-100 
>>> <https://github.com/apple/swift-evolution/blob/master/proposals/0100-add-sequence-based-init-and-merge-to-dictionary.md>
>>>  is a proposal to add an init to Dictionary from a sequence of key/value 
>>> pairs. It’s a commonly requested feature, and IMO much needed and should be 
>>> added as soon as we move to the appropriate phase in Swift’s evolution. 
>>> 
>>> Another commonly requested Dictionary feature is similar: add a 
>>> Dictionary.init that takes a sequence, and a closure that maps that 
>>> sequence to keys. This is useful, for example, when you have a sequence of 
>>> objects that you frequently need to index into via one property on those 
>>> objects, so you want to build a fast lookup cache using that property.
>>> 
>>> Now, if we implement SE-100, that second request can be easily composed. It 
>>> would be something like Dictionary(sequence.lazy.map { (key: 
>>> $0.someProperty, value: $0) } )
>>> 
>>> Some people look at that line of code and think sure, that’s what I’d do 
>>> and it’s easy enough that the second helper shouldn’t be added as it’s 
>>> superfluous. Others look at it and say that it is unreadable clever-code FP 
>>> nonsense, and we should just add the helper method because most programmers 
>>> wouldn’t be able to read or write that easily.
>>> 
>>> As we expand (and maybe contract :) the standard library, this kind of 
>>> question comes up a lot, so it is worth setting out some criteria for 
>>> judging these
>>> “helper” methods. Here’s my take on such a list (warning: objectivity and 
>>> subjectivity blended together in the below).
>> 
>> This is a great analysis and list of criteria.  Thanks for putting this 
>> together Ben!
>> 
>>> 
>>> 1. Is it truly a frequent operation?
>>> 
>>> The operation needs to carry its weight. Even once we have ABI stability, 
>>> so the size of the std lib becomes less of a concern as it could ship as 
>>> part of the OS, we still need to keep helper method growth under control. 
>>> APIs bristling with methods like an over-decorated Xmas tree are bad for 
>>> usability. As mentioned in the String manifesto, String+Foundation 
>>> currently has over 200 methods/properties. Helpers are no good if you can’t 
>>> find them to use them.
>>> 
>>> Someone mentioned that they actually don’t find themselves using 
>>> enumerated() all that often. I suspect enumerated in its current form isn’t 
>>> all that useful. In a quick (and non-scientific) review of search results 
>>> for its use on GitHub, nearly all the examples I see of it are either 1) 
>>> misuse – see more below, or 2) use of it to perform the equivalent of 
>>> in-place map where the index is used to subscript into the array and 
>>> replace it with a transformed element.
>>> 
>>> I think the std lib needs an in-place map, and if enumerated() is removed, 
>>> this one most-common use case should be added at the same time.
>> 
>> I just did a quick search in a couple of projects and found a handful of 
>> good examples of valid uses of `enumerated`.  I have simplified the examples 
>> from real world code, but I think they demonstrate the kinds of things this 
>> is good for.
>> 
>> // only show the first 5 views
>> for (i, view) in views.enumerated() {
>>     view.hidden = i >= 5
>> }
>> 
> 
> Interesting that several of these rely on reference types as elements. This 
> does make the loops a lot simpler. In-place amendment of an array would be 
> messier, and the combination with zip definitely pushes back against my 
> notion that an in-place map would be a useful alternative (since you couldn’t 
> mutate the mutable half of the zipped collection in-place without us jumping 
> through some hoops).
> 
> 
>> // apply alternating view background colors
>> for (i, view) in views.enumerated() {
>>     view.backgroundColor = i % 2 ? bgColor1 : bgColor2
>> }
>> 
> 
> This is an interesting one because it highlights something else I think is 
> missing from the std lib: Sequence.cycle, which would make an infinite 
> sequence by repeating a sequence, enabling what I think is probably a 
> readability win in some cases:
> 
> let colors = [bgColor1, bgColor2].cycle
> for (color, view) in zip(colors, views) {
>       view.backgroundColor = color
> }
> 
> Also i%2 used as a boolean – what language is this? :)

Oops!  :)

> 
>> // linear layout
>> for (i, view) in views.enumerated() {
>>    let x = width * CGFloat(i)
>>    view.frame = CGRect(x: x, y: 0, width: width, height: height)
>> }
>> 
>> // deriving locations for an equally spaced gradient
>> let locations = colors.enumerated.map { CGFloat($0.0) / CGFloat(colors.count 
>> - 1) }
>> 
> 
> This one feels like it’s an example of enumerate discouraging use of stride, 
> which hits cases 2: readability and 4: performance (unlike the others, where 
> the alternatives are no better or worse) 

That’s fair.

> 
>> There are other ways to accomplish similar things (use `prefix` and 
>> `dropFirst` comes to mind), but many reasonable people would argue that 
>> using `enumerated` is a very straightforward way to do a lot of things.
>> 
> 
> Totally. Enumerating things is certainly sometimes useful, I just don’t think 
> it’s useful enough to have a dedicated but inflexible method to do it.
> 
>>> 
>>> 2. Is the helper more readable? Is the composed equivalent obvious at a 
>>> glance?
>>> 
>>> When an operation is very common, the big win with a helper method is it 
>>> creates a readable vocabulary. If enumerated() is very common, and everyone 
>>> sees it everywhere, it makes that code easier to read at a glance.
>>> 
>>> That said, I think that the alternative – zip(0…, sequence) – is just as 
>>> readable once you are familiar with zip and ranges, two concepts that, IMO 
>>> at least, it is important that every Swift programmer learn about early on. 
>>> I would even go so far as to say that enumerated is harmful if it 
>>> discourages new users from discovering zip.
>> 
>> This is a very good point.  Requiring programmers to zip with a range could 
>> help them to learn to think a little bit differently and learn more general 
>> tools.  That’s a good thing.  
>> 
>> The other thing it does is make the order of the index and the value more 
>> clear at the call site.  I think this is a very important point.  I always 
>> forget which order `enumerated` uses every time I reach to use it.
>> 
>> Of course the current alternative is actually `zip(0..<sequence.count, 
>> sequence)` which is not so great.  
>> 
> 
> Totally. zip replacing enumerated will only fly in combination with one-sided 
> ranges.
> 
>> In order to make `zip(0…, sequence)` a workable alternative we would need to 
>> grab `…` as a unary postfix operator which produces a sequence by 
>> incrementing values.  Is that something we want to do?  If the answer turns 
>> out to be yes, then I am in agreement with your analysis.  The composed 
>> alternative would be superior and `enumerated` wouldn’t carry it’s weight.
>> 
> 
> I think based on the String discussions there’s fairly well-established 
> support for one-sided  …

Agree, but that conversation feels like it hasn’t fully resolved yet.

The primary concern I have with one-sided … is whether it would limit our 
options for variadics and tuple unpacking in a way we would come to regret.  I 
know that was mentioned during the String discussion but I didn’t get a sense 
that this question was explored all they way through yet.  

But maybe I missed some details - I haven’t been following every aspect of the 
String discussion and it’s possible I missed a few posts I would have been 
interested in, including this topic.

> 
>>> 
>>> OTOH, an example that I think is strongly justified on readability grounds 
>>> is Sequence.all(match:), a function that checks that every element in a 
>>> sequence matches a predicate. This is by far the most common extension for 
>>> Sequence on GitHub. Yes, you can write this as !seq.contains(!predicate) 
>>> but that far less readable – especially since it requires you write it with 
>>> a closure that !’s the predicate i.e. !seq.contains { !predicate($0) }, 
>>> because we don’t have a ! for composing with predicate functions (though 
>>> maybe we should).
>>> 
>>> 3. Does the helper have the flexibility to cover all common cases?
>>> 
>>> This, I think, is where enumerated() really starts to fall down.
>>> 
>>> Sometimes you want to start from not-zero, so maybe it should be 
>>> Sequence.enumerated(startingFrom: Int = 0)
>>> Sometimes you want non-integer indices so maybe we should add indexed().
>>> Sometimes you want it the other way around (not for a for loop but for 
>>> passing the elements directly into a mapping function) so now you need a 
>>> flip.
>>> Sometimes you want to enumeration to run backwards in some way.
>>> 
>>> Less of a problem if you’re already accustomed to composing your 
>>> enumeration:
>>> 
>>> Enumerate starting from 1: zip(1…, c)
>>> Enumerate indices: zip(c.indices, c)
>>> Need the pair to be the other way around: zip(c, 0…)
>>> Need the enumeration to run the other way: zip((0..<c.count).reversed,c) or 
>>> zip(0…,c).reversed() or zip(0…,c.reversed()).
>>> 
>>> Similarly, for the Dictionary helper – what if you want a sequence, and a 
>>> closure to map keys to values, rather than values to keys?
>>> 
>>> (zip also needs to retain its collection-ness, which is filed as SR-3760, a 
>>> great starter proposal if anyone wants to take it on :)
>>> 
>>> 4. Is there a correctness trap with the composed equivalent? Is there a 
>>> correctness trap with the helper?
>>> 
>>> As others noted on the thread, enumerated() used for indices encourages a 
>>> possible correctness error:
>>> 
>>> for (idx, element) = someSlice.enumerated() { } // slices aren’t zero based!
>>> 
>>> Now, chances are that since out-of-bounds subscripting traps on arrays, 
>>> this bug would be caught early on. But maybe not – especially if you ended 
>>> up using it on an UnsafeBufferPointer slice (perhaps dropped in for 
>>> performance reasons to replace an Array) and now all of a sudden you have a 
>>> memory access violation that could go undetected.
>>> 
>>> On the flip side: many operations on collections that use indices are hard 
>>> to get right, especially ones that involve mutating as you go, like 
>>> removing elements from a collection based on some criteria, where you have 
>>> to work around index invalidation or fencepost errors. For this reason, the 
>>> std lib probably ought to have a RangeReplaceableCollection.remove(where:) 
>>> method so people don’t have to reinvent it and risk getting it wrong.
>>> 
>>> 5. Is there a performance trap with the composed equivalent? Or with the 
>>> helper?
>>> 
>>> The composed example of Dictionary from a sequence+closure, you need to 
>>> remember the .lazy part in sequence.lazy.map to avoid creating a temporary 
>>> array for no reason. A helper method might lift that burden from the user. 
>>> remove(where:) can easily be accidentally quadtratic, or perform needless 
>>> element copies when there’s little or nothing to remove.
>>> 
>>> Counter example: the fact that map is eager and chaining it creates 
>>> temporary arrays needlessly is a performance problem caused by introducing 
>>> it as a helper method.
>>> 
>>> 6. Does the helper actually encourage misuse?
>>> 
>>> This is a very common pattern when searching GitHub for uses of 
>>> enumerated(): 
>>> 
>>> for (index, _) in collection.enumerated() { 
>>>     mutate collect[index] in-place i.e. collection[index] += 1
>>> }.
>>> 
>>> (or even they assign the element then don’t use it, for which specific case 
>>> we don’t currently emit a warning)
>>> 
>>> What the user really needs is just:
>>> 
>>> for index in collection.indices { etc. }
>>> 
>>> I expect if the standard way to do enumerated was with zip, people wouldn’t 
>>> do this as often. In-place map would be even better.
>>> 
>>> 8. Can a native implementation be made more performant than the equivalent 
>>> composition?
>>> 
>>> Dictionary.mapValues(transform: (Value)->T) can be implemented internally 
>>> much more efficiently than just composing it with map and the key/value 
>>> sequence initializer, because the layout of the hash table storage can be 
>>> re-used in the new dictionary, even when the Value type is different.
>>> 
>>>> On Jan 31, 2017, at 6:24 AM, Chris Eidhof via swift-evolution 
>>>> <[email protected] <mailto:[email protected]>> wrote:
>>>> 
>>>> Hey everyone,
>>>> 
>>>> I've organized a number of Swift workshops over the last two years. There 
>>>> are a couple of things that keep coming up, and a couple of mistakes that 
>>>> I see people making over and over again. One of them is that in almost 
>>>> every workshop, there's someone who thinks that `enumerated()` returns a 
>>>> list of (index, element) pairs. This is only true for arrays. It breaks 
>>>> when using array slices, or any other kind of collection. In our 
>>>> workshops, I sometimes see people doing something like 
>>>> `x.reversed().enumerated()`, where `x` is an array, and somehow it 
>>>> produces behavior they don't understand.
>>>> 
>>>> A few ways I think this could be improved:
>>>> 
>>>> - Move enumerated to Array
>>>> - Change enumerated to return `(Index, Iterator.Element)` (this would mean 
>>>> we at least need to move it to collection)
>>>> - Remove enumerated
>>>> - Keep things as is
>>>> 
>>>> In any case, just wanted to share my experience (gained from teaching 
>>>> people). 
>>>> 
>>>> -- 
>>>> Chris Eidhof
>>>> _______________________________________________
>>>> swift-evolution mailing list
>>>> [email protected] <mailto:[email protected]>
>>>> https://lists.swift.org/mailman/listinfo/swift-evolution 
>>>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
>>> 
>>> _______________________________________________
>>> swift-evolution mailing list
>>> [email protected] <mailto:[email protected]>
>>> https://lists.swift.org/mailman/listinfo/swift-evolution 
>>> <https://lists.swift.org/mailman/listinfo/swift-evolution>
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to