[Review] Require self for accessing instance members


(Colin Cornaby) #1

+1 to this proposal (again)

* What is your evaluation of the proposal?

I know that adding this requirement is something that could frustrate a number of people, both now and people who adopt Swift in the future... But I feel that the shadowing problem that it solves could be a significant issue for developers who are new. I also feel that it matches the idea behind Swift to encourage safe development practices. Concepts like optionals also are extra additions to the language that can frustrate and annoy developers, but in the end create safer and more clear code.

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

I feel Swift should have some sort of handling of this sort of shadowing. Unless a better proposal comes along to solve the same problem, I feel this problem is significant enough to warrant the change.

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

See notes above. I feel like this fits will with the direction of Swift in adding new language requirements to force a developer to deal with common coding errors.

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

It was noted in an earlier thread that this feature was added to Obj-C not intentionally, but as a result of a collection of issues. I actually appreciated this feature of Obj-C and felt like it made Obj-C a more exacting language. I know there are similar complaints about Obj-C's verbosity, but I feel like this proposal outweighs that with the additional clarity that would be added.

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

I've been actively watching this review and I've hit this problem a few times in Swift. I understand that this issue is contentious, and that if this change happens, people might still be complaining years and years from now about it. But it reduces confusion in the language and eliminates possible developer error, which seems to align with the goals of Swift.

···

On Dec 16, 2015, at 10:55 AM, Douglas Gregor via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of “Require self for accessing instance members” begins now and runs through Sunday, December 20th. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0009-require-self-for-accessing-instance-members.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

* 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 you 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?

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/master/process.md

Cheers,
Doug Gregor
Review Manager


(Tino) #2

I don’t like that proposal, but if self becomes mandatory, I really hope that prefixes like „m“ or „_“ for member variables are officially forbidden :wink:

There is already one proposal ("method cascading") that is quite the opposite of this one (already mentioned somewhere… it's really hard to keep an overview) — and another one that is even more contrary:
https://lists.swift.org/pipermail/swift-evolution/Week-of-Mon-20151214/002628.html

Someone should create a bugtracker where you can vote weather a "bug" should be fixed or extended :wink:


(Jeremy Pereira) #3

-1 to this proposal

  • What is your evaluation of the proposal?

I think the proposal addresses a minor problem in the wrong way. I think it seeks to solve the issue of shadowing variables by forcing a particular coding style on programmers.

There is at least one error of fact in the proposal "as is intrinsically the case in Objective-C”. This is not the case with Objective-C instance variables where “self->ivar” is not mandatory.

I also disagree with the assertion that it is "more readable at the point of use”. I think that is entirely subjective and I and several other participants have disagreed with this assertion.

I agree that it is “more consistent than only requiring self in closure contexts”. However, the use of optional self in some situations and not others actually aids readability. The often cited instance is in closures to remind the programmer that self is captured.

It may be fractionally less confusing from a learning point of view, but I think a programmer that cannot grasp this is going to struggle with many of the more complex language features anyway. Other languages adopt the same pattern as Swift (e.g. Java, C++, Objective-C for instance variables) and learners in these languages do not seem to have problems in this respect.

I think this proposal will lead to anti-patterns like the following because people will want to get rid of all the “unsightly” selfs:

    struct Point
    {
  var x: Int
  var y: Int

  func doSomethingWithXAndY()
  {
      var x = self.x
      var y = self.y
      // Do complicated stuff with the local variables
        }
    }

The above could be, in itself, a source of bugs e.g. changing the local x and y and forgetting to write them back to the properties.

Finally, the "Alternatives Considered" section does not consider any alternatives other than status quo. There is at least one alternative to resolve the issue the proposal seeks to address.

I’m also concerned that the “Community Responses” section includes only the positive feedback. There was quite a lot of negative feedback on the list too. Where is it?

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

Yes. However, the proposed solution is wrong. I would suggest a compiler warning for when a property or global is shadowed by a local variable would be sufficient.

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

No. As a rule, the drive with Swift is to remove unnecessary boilerplate. This is adding it in.

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

Only Javascript, but I find it really clunky in Javascript. I have done a lot of Java and C++ programming both of which have implicit this. I have never felt the need to have explicit this in those two languages except in certain well defined situations e.g. in the constructor where the parameters shadow the instance variables.

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

I have done a lot of reading of the thread on the swift-evolution list. Other than that, only the time to write this response.