guard let x = x


(Jon Hull) #1

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 :slight_smile:
    // 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.