[swift-evolution] [swift-evolution-announce] [Review] SE-0023 API Design Guidelines

[David Owens II]

Let’s try again with less of a sleepy-sick stupor. =)

My concern is that the name of the function is the only indicator for non-struct types when looking at the signature. For structs, we can get more assurance; e.g. the bug demonstrated below cannot happen. However, that doesn’t mean the opposite bug is avoidable: return a copying instead of mutating self.

The problem I was trying to illustrate, and did a really poor job at, is that the convention doesn’t actually provide us with any guarantees that our implementations are correct. So when I say this:

> I guess my point is this: codifying a convention seems pre-mature as that convention doesn't bring the safety goals of the language into a place that's verifiable. All of the other guidelines are simply about clarity of use, this convention has a far reaching impact.


Here’s a demonstration of when the guidelines was applied incorrectly, either due to ignorance of the guidelines or simply a copy/paste type bug.

var bar = Bar(items: [2, 1, 3])
let filteredBar = bar.filter { $0 != 3 }

bar.items                      // output: [2, 1 3]
filteredBar.items              // output: [2, 1]

let filteredSortedBar = bar
    .filter { $0 != 3 }
    .sort()

bar.items                      // output: [2, 1, 3]
filteredSortedBar.items        // output: [1, 2]

let sortedFilterBar = bar
    .sort()
    .filter { $0 != 3 }

bar.items                      // output: [1, 2, 3] WHAT?
sortedFilterBar.items          // output: [1, 2]

var expected = Bar(items: [2, 1, 3])
expected
    .filterInPlace { $0 != 3 }
    .sortInPlace()

expected.items                 // output: [1, 2]

When reading the code with an understanding of the guidelines, this code is misleading as to what it should be doing. That’s what I find dangerous. Of course, we can argue on the merits of these types of bugs, but we’ve all seen conventions misused and it sucks.

If this convention is just a stop-gap until a language feature can be built to help with this, then ok. 

-David


sample implementation that demonstrates the bug that gets us out-of-sync with the guidelines:

public class Bar<T : Comparable> {
    var items: [T]
    
    init(items: [T]) {
        self.items = items
    }
    
    func sortInPlace() -> Self {
        self.items.sortInPlace { $0 < $1 }
        return self
    }
    
    func sort() -> Self {
        self.items.sortInPlace { $0 < $1 }
        return self
    }
    
    func filterInPlace(includeElement: (T) -> Bool) -> Self {
        self.items = self.items.filter(includeElement)
        return self
    }
    
    func filter(includeElement: (T) -> Bool) -> Bar<T> {
        let newItems: [T] = self.items.filter(includeElement)
        return Bar(items: newItems)
    }
}

> On Jan 24, 2016, at 7:18 PM, Dave Abrahams <dabrahams at apple.com> wrote:
> 
> 
> on Sun Jan 24 2016, David Owens II <david-AT-owensd.io> wrote:
> 
>> Sorry, I meant to add the do() version too in order to show the
>> difference better. /sigh
> 
> do() version?
> 
>> 
>>> On Jan 24, 2016, at 4:53 PM, David Owens II via swift-evolution
>>> <swift-evolution at swift.org> wrote:
>>> 
>>> 
>>>>> I guess my point is this: codifying a convention seems pre-mature as
>>>>> that convention doesn't bring the safety goals of the language into a
>>>>> place that's verifiable. All of the other guidelines are simply about
>>>>> clarity of use, this convention has a far reaching impact.
>>>> 
>>>> Sorry, could you clarify what you mean by "bring the safety goals of the
>>>> language into a place that's verifiable" and clarify why having a "far
>>>> reaching impact" would somehow conflict with being "about clarity of use?"
>>>> 
>>>> It seems to me that this convention is about how to express whether a
>>>> method is going to mutate so it's clear at the use-site.  What am I
>>>> missing?
>>> 
>>> 
>>> The problem is it's unclear to me whether you mean mutate in the
>>> true sense of the word or only applied to a struct with a function
>>> annotated with the mutating keyword.
>>> 
>>> The naming convention provides no safety when dealing with
>>> non-struct types as we cannot enforce that a method on a class does
>>> not mutate it's internal members.
>>> 
>>> That's the clarity I'm looking for.
>>> 
>>> Given this API set:
>>> 
>>> protocol InPlaceable {
>>>    mutating func doInPlace()
>>> }
>>> 
>>> public struct Foo: InPlaceable {
>>>    mutating func doInPlace() {}
>>> }
>>> 
>>> public class Bar: InPlaceable {
>>>    func doInPlace() {}
>>> }
>>> 
>>> var lie: InPlaceable = Bar()
>>> lie.doInPlace()
>>> 
>>> let lie2 = Bar()
>>> lie2.doInPlace()
>>> 
>>> The convention will tell us a lie unless we are extremely
>>> careful. It's this lie that concerns me. We cannot guarantee that
>>> the "doInPlace" truly matches the definition we are seeking.
>>> 
>>> 
>>> -David
>>> _______________________________________________
>>> swift-evolution mailing list
>>> swift-evolution at swift.org
>>> https://lists.swift.org/mailman/listinfo/swift-evolution
>> 
> 
> -- 
> -Dave

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20160125/583f88c4/attachment.html>