[Source Break] Making all collection/sequence wrappers base properties internal

[LucianoPAlmeida]

Making all collection/sequence wrappers base properties internal

Fixes #84

cc @natecook1000 @timvermeulen

Checklist

• I've added at least one test that validates that my change is working, if appropriate
• I've followed the code style of the rest of the project
• I've read the Contribution Guidelines
• I've updated the documentation if necessary

[xwu]

Does this need to be made internal? Is there any benefit to making it visible?

[LucianoPAlmeida]

This is a really good question, the only problem I can think is where Base.Element is a reference or we make set public, exposing it may allow users to modify state causing the wrapper collection to change behavior, but I'm not sure if this is really a problem ...

Is there any benefit to making it visible?

I can't think of an use case where someone could need access to this property.

[xwu]

You can't set a stored let.

[LucianoPAlmeida]

Ah! indeed you right, so the only concern would be references then e.g.

struct CollectionWrapper<Base> where Base: Collection {
  let base: Base
  let separator: Base.Element
}

class Ref: Equatable {
  var b: Bool = true
  
  init() {}
  init(_ b: Bool) { self.b = b }
  
  static func == (lhs: Ref, rhs: Ref) -> Bool {
    return lhs.b == rhs.b
  }
}

let l: [Ref] = [Ref(), Ref(false), Ref()]

var c = CollectionWrapper(base: l, separator: Ref(false))

print(c.separator.b)

c.separator.b = true
// Any operation on `c` after that may have behavior changed 

print(c.separator.b) 

Although is very specific, this would be possible if we have the public property and can lead to unexpected behavior
But I still not sure if we should consider this really a problem here ... WDYT?

[xwu]

I'm not sure I see the problem. The same behavior would happen if the reference were mutated not via c.separator: that's just what it means to have a reference type.

Overall, while I don't have a concrete use case in mind, I do rather think it's reasonable for a user to want to know what separator they are using here, and to query that through the type that's using the separator. It is a property that's taken into account as part of the type's equivalence relation, is it not?

[timvermeulen]

Overall, while I don't have a concrete use case in mind, I do rather think it's reasonable for a user to want to know what separator they are using here, and to query that through the type that's using the separator.

@xwu Would you say the same argument could be made when deciding whether to make the base collection private, and if not, what makes it different?

[xwu]

The rationale for removing the public accessibility of base is given in issue 84. As noted above, this does not apply to separator or to size below.

[timvermeulen]

Sure, I was just curious whether you thought your reasoning would also extend to the base property if we ignore the concern about substitutability. Do you think it's reasonable for a user to want to access the base collection?

[LucianoPAlmeida]

I was thinking 2 things after @timvermeulen comments

• Does it make sense to be able to access what separator or size the wrapper collection is using if they can't access the base collection?
• I was wondering that for the user perspective the most important thing is that this is a collection that behave as expected, kind of in the same sense that an eager implementation that just produces a result array. So by this reasoning specific properties of the wrapper type and I'm not sure but maybe even the wrapper type itself are kind of transparent for the user in the sense that they probably won't even expect to have access to those kind of specific collection properties. This is more a question than an affirmation, I'm still thinking about it... WDYT? @xwu @timvermeulen

[xwu]

Sure, I was just curious whether you thought your reasoning would also extend to the base property if we ignore the concern about substitutability. Do you think it's reasonable for a user to want to access the base collection?

My only concern is about substitutability.

[xwu]

Ditto here: does size need to be made internal?

[xwu]

@LucianoPAlmeida I take it the 👍 indicates that you'd agree with reverting this change?

[LucianoPAlmeida]

Ah sorry, I just thought that it was the same topic as the on going discussion above... that's why I just reacted and didn't respond here :)

[natecook1000]

@swift-ci Please test