[swift-evolution] [Review] SE-0081: Move where clause to end of declaration

[Nicola Salmoria]

David Hart via swift-evolution <swift-evolution at ...> writes:

> 
> 
> Hi Karl,
> 
> As author of this proposal, the one about constraints on associated types,
and the one on type-aliases in protocols (all from the Generics Manifesto -
original authorship to Douglas Gregor) I’d like to provide additional
reasoning behind my wish to push this proposal through, as a whole.
> 
> First of all, there is a personal preference. I’ve used C# for many many
years, which has its where clause at the end of the declaration (like this
proposal), and I’ve used Swift since its unveiling. The experience with
using both styles for several years makes me favour this proposal’s syntax
because I find it much easier to read and format for readability.
> 
> Constraints on associated type will provide more expressivity but I doubt
it will greatly reduce protocol constraint clauses in a majority of cases.
And yes, type-aliases in protocols will shorten clauses, but I still think
they will more readable with the where clause at the end.
> 
> For example, here is a method I took (as-is) from the Standard Library
which has a few constraints, and which I further simplified if we imagine
that Sequence has an Element typealias for Iterator.Element:
> 
> 
> internal func _arrayOutOfPlaceReplace<
>  B : _ArrayBufferProtocol, C : Collection
>  where  C.Element == B.Element,
>  B.Index == Int
> >(  _ source: inout B, _ bounds: Range<Int>, _ newValues:
> C, _ insertCount: Int) {
> 
> See how the Standard Library authors formatted it for readability and how
as a consequence arguments which use the generic types are further apart
from the declaration of those generic types. But with this proposal, the
declaration might be formatted to:
> 
> internal func _arrayOutOfPlaceReplace<B,C>(_ source: inout B,
> _ bounds: Range<Int>, _ newValues: C, _ insertCount: Int)
>  where
>  B : _ArrayBufferProtocol,
>  C : Collection,
>  C.Iterator.Element == B.Element,
>  B.Index == Int
> {
> 
> Do you need believe this is now more readable?
> 
> David.

Thanks for the real world example!

I think that the second arguably *looks* better. But is it more *readable*?
Not for me.

Let me try to do a brain dump while I mentally parse the declaration.

> internal func _arrayOutOfPlaceReplace<B,C>(

Mmm, here we have a function that has something to do with arrays, generic
on two types. Maybe it will take an Array<B> and output an Array<C>?

> _ source: inout B,

Ah, the first argument is a B that will be changed. Maybe the function deals
with Array<B> and will replace occurrences of 'source' with something else?

> _ bounds: Range<Int>,

Ok, this surely must be the range of the Array to operate on.

> _ newValues: C,

Oh, this parameter is called 'newValues', so C can't be a single value. It
must be a Collection, surely.

> _ insertCount: Int)

... I can't figure out what this could be.

>  where
>  B : _ArrayBufferProtocol,

Oh, B was some kind of buffer. OK so the function probably takes this
buffer, and replaces 'range' with 'newValues'

>  C : Collection,
>  C.Iterator.Element == B.Element,

Good, this confirms what I thought.

>  B.Index == Int

Looks like an implementation detail, I guess that's why 'range' is a
Range<Int> and 'insertCount' is an Int. Presumably the function needs to do
some operations that the generic Index doesn't guarantee.

So that's the end of it. I have a vague idea of what the function might do,
and had to revise my expectations a few times while reading its declaration.


Let's try again with the original declaration.

> internal func _arrayOutOfPlaceReplace<
>  B : _ArrayBufferProtocol, C : Collection

OK, here we have a function that deals with Arrays, generic on some buffer
thing and on a Collection. B is presumably what it needs to operate on, and
C will be the thing to replace with.

>  where  C.Element == B.Element,

Indeed thic confirms that B and C must be compatible.

>  B.Index == Int

Looks like an implementation detail, presumably we need to indicate the
position in the array where to do the replace.

> >(  _ source: inout B,

OK so this is a mutating function, and B is a buffer that will be modified.

> _ bounds: Range<Int>,

This is probably the range of the buffer that needs to be replaced, indeed
we had B.Index == Int.

> _ newValues: C,

And this collection contains what we need to insert in the buffer.

> _ insertCount: Int) {

Not sure what this might be, maybe the function will copy only the first
'insertCount' elements of C.

There we are. I read the declaration of the function from beginning to end
and gradually formed a rough understanding of it without needing to change
my expectations halfway through. I still have doubts about 'insertCount',
but I was at least able to formulate an hypothesis about its use.

YMMV, but as far as I'm concerned, the original declaration was much easier
to understand.

Now if we had generic typealiases, we could do something like

typealias CollectionOf<T> = protocol<Collection> where Collection.Element == T

and if my intuition about B.Index is right, the declaration could be further
simplified to something like this

internal func _arrayOutOfPlaceReplace<
  B : _ArrayBufferProtocol, C : CollectionOf<B.Element>
  where B.Index == Int
>(  _ source: inout B, _ bounds: Range<B.Index>, _ newValues: C, _
insertCount: B.Index) {

Now *that* is more readable.


Nicola