An implicitly-declared variable is not an implicit behavior. That didSet { }
always calls the getter today is both implicit and unexpected. No one is contesting this proposed change. However, referencing oldValue
in the body is explicit. I fail to understand why anyone should be surprised that the getter is invoked.
I agree with those who have argued that deprecating oldValue when it is not explicitly declared is inconsistent and unnecessary.
Using oldValue obviously requires a get. Not using it obviously doesn't require a get - except that there is a bug in current versions of the compiler.
Jeremy
We keep talking about how
didSet(oldValue)
makes performance implications clear or explicit but I fail to see how that's the case. In my opinion, only documentation can make it clear what the implications are.
My thoughts exactly
I thought I would survey my own code for didset
blocks. Among dozens of didSet
blocks I found only a single use of oldValue
. It was an implicit use. I did find one class with a dozen or so didSet
blocks like the following:
didSet(newValue) { // code that didn't reference newValue or oldValue }
FWIW, I inherited that code but I certainly copied and pasted some of the existing didSet blocks to make new ones. I never noticed that the parameter name was reversed from what it should be.
What I think will happen if this change is implemented is that there will be lots of code that ends up with the oldValue parameter but doesn't use it. Unless there's a lint rule for this that gets widely used the fetching/non-fetching behavior will simply be ignored. Swift doesn't normally warn about unused parameters and there's not normally any negative side effect of not using parameters. didSet
apparently will be a special case that violates this rule. I would frankly assume that unused parameters are optimized away, and I would be wrong in this case. I understand that side effects of getters need to be taken into account and I guess that's why the fetching behavior would have to be done.
And yes, I removed all my unused newValue parameters in order that my code doesn't fetch the oldValue unnecessarily in the future.
I'm in favor of leaving implicit oldValue the way it is.
I think someone suggested further up the thread that we could add another warning for unused oldValue
which could be useful in the scenario you described.
Well, the inconsistency is in changing the semantics then. There’s no proposal to get rid of the implicitly declared parameters in willSet
and set
because whether or not you use them has no performance or behavior implications.
If consistency were actually that crucial, shouldn’t we reject the whole proposal?
Can you elaborate on what you mean by this? I’m not sure what is inconsistent about the semantics.
See this comment. It is not feasible to give willSet
without newValue
the same treatment as didSet
without oldValue
. So why should the syntax rules be “consistent”?
I think it would be more surprising to a user why they can implicitly call newValue but not oldValue. To me that would feel inconsistent. There is already a semantic difference between the two in that one is before setting and one is after setting, so I don’t see a benefit to changing the syntax. On the other hand there is a visual and behavioral consistency in keeping the way you access the property observers identically shaped.
For me, the consistency of both having “observer” semantics is why they should both have the same syntax, while whether or not the oldValue is fetched feels like an implementation detail.
46⁄142 or roughly one third of the posts in this thread (#97 onward) were made after the review period was scheduled to have closed (December 9). Is this thread even supposed to still be open?
Those are not at all comparable. When a type has an instance property which is not mentioned at all in a method body, no one expects the getter for that property to be invoked when the method is called. The behavior of didSet
violates this expectation. The main thrust of this proposal is to fix this unexpected inconsistency. The performance aspects are important, but not the only (or main?) motivation.
For willSet
, an attempt to optimize would lead to an internal inconsistency in behavior, but in either case, the behavior of willSet
does not violate any expectations today.
Review threads are typically left open until the announcement thread is created so there’s a place to continue the discussion. A few times crucial feedback has come after the review technically ended.
Precisely. You have detailed why willSet
and didSet
are not comparable. You have given an excellent description of how it’s not just about performance but also about semantic differences, and how the implicit variable causes confusion in didSet
but not willSet
. Hence my point that the “consistency” argument here against changing syntax doesn’t hold water.
Fine. But expectations are important, too. There's no expectation that a method body with zero references to a property should read the property.
On the other hand, willSet
is presaging a change to the property. When value semantics are in play, this is typically done as read-modify-write(back), and only optimizations alter that flow (in-place modification). So there's no behavioral expectation being violated when the optimization can't be done.
I'm +1 on the proposal. I'm somewhat ambivalent on the deprecation of implicit oldValue. I would have no issue moving to something more explicit (especially as it has a behavioural impact), but I feel that if that change was to be made it would also need to be made to both set
and willSet
for consistency (and should ideally be made at the same time). That said, sticking with the implicit oldValue will certainly save a lot of time for developers and confusion for those looking at older sample code and tutorials online.
One thing I haven't seen covered (though I may have just missed it) is regarding the naming of the argument in the the explicit didSet(oldValue)
scenario. It's a minor detail, but would this imply that the oldValue
variable would be able to be called anything, as with any function? Or would it still be required to be oldValue
?
It already can be named anything, and this won't change. You can even name it newValue
if you want to confuse everyone (someone did this earlier in the thread, most likely by mistake).
Yes, you can call it anything you want (but the fix-it attached to the deprecation warning is going to use oldValue
since that's what the implicit variable is called).
*If the core team decides to deprecate the implicit oldValue
*ducks for cover*
Review Conclusion
Thanks to everyone for the feedback. Given the arguments against the deprecation warning, the core team has decided to accept the original proposal, of altering the fetch semantics without deprecating the implicit oldValue
.
Thanks to everyone for the thoughtful discussion of the merits.
Ben