[swift-evolution] guard let x = x
Jonathan Hull
jhull at gbis.com
Tue Nov 1 19:05:49 CDT 2016
I am a little worried about shadowing as well (isn’t that why we don’t allow ‘let x’… even though it seems a bit more obvious to me with that syntax that there is shadowing?)
Have we considered doing exactly what people naively expect? That is, instead of creating a shadow constant/variable, we have unwrap cause the variable to be implicitly unwrapped within the scope of the if/guard statement? In other words, it is still the same variable, but it has been safely implicitly unwrapped, so you don’t have to worry about the optional. The compiler should be able to optimize well enough with the knowledge it has (that it is actually non-nil… especially if it isn’t modified in that scope) that it shouldn’t add a performance hit...
Thanks,
Jon
> -1 on
>
> if unwrap myValue { ... }
>
>
> In my opinion, *shadowing names is an anti-pattern,* so making it easier
> isn't something we should encourage. Detailed examples, shall you wish, are
> below.
>
> Consider a developer who is suddenly able to change all employment
> relationships in the company.
> He's happily coding a function that should guarantee him a boss-free
> lifestyle:
>
> class Employee {
> var currentSupervisor: Employee?
>
> // Precondition: you can do whatever you want :)
> // Requirements: currentSupervisor == nil at the end
> func fireYourBoss() {
> if unwrap currentSupervisor {
> company.goToMeeting() // <- anything can happen on a meeting
> company.fire(currentSupervisor) // <- firing a manager
> nullifies the supervisor relationship
> // Now I definitely have currentSupervisor == nil... or do I?
> }
> }
> }
>
> This code contains a design problem: the local variable bound to
> currentSupervisor name shadows the instance property with the same name.
> The unwrap operator guarantees that the values bound to those names are
> equal in the beginning of the then-block; however, there is no reason to
> think that this will hold after company.goToMeeting().
>
> Compare this to how this code will read without the shadowing:
>
> if let bad_boss = currentSupervisor {
> company.goToMeeting()
> company.fire(bad_boss)
> }
>
> It is much clearer in this case what happens – the person being fired is
> the old value of currentSupervisor. Since the problem is now clearly
> diagnozed, the code can now be improved to guarantee the blissful future,
> for example as follows
>
> if let bad_boss = currentSupervisor {
> company.goToMeeting()
> if let another_boss = currentSupervisor {
> company.fire(another_boss)
> }
> }
>
> Similarly, in general, shadowing is a big source of bugs and should be
> avoided in all cases where the shadowed and shadowing variable's values can
> be different.
>
> In multithreaded programs it's basically only safe to shadow those names
> that are bound with let, so we could implement unwrap for those, but it's
> unclear what the big benefits of it are.
>
> Thanks for reading up to here,
> Ilya.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20161101/4cea307b/attachment.html>
More information about the swift-evolution
mailing list