New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variable name for Optional Binding #64

Merged
merged 1 commit into from Dec 11, 2014

Conversation

Projects
None yet
@gregheo
Member

gregheo commented Dec 9, 2014

To use the value of an optional property, we can use Optional Binding. For example:

if let t = self.title {
  // do many things with title
}

But I'm always confused about the naming of the optionally bounded variable. In the case above, how should the variable t be named? Using the first letter is of course very bad!

Any ideas?

@sammyd

This comment has been minimized.

Show comment
Hide comment
@sammyd

sammyd Nov 23, 2014

Contributor

I often find myself shadowing the original variable, but I am in two minds as to whether this constitutes good style or not:

if let title = title {
   // do something with title
}

I like that we're not making up pointless new variable names (e.g. unwrappedTitle), however, I think there are times that shadowing can be confusing.

I am finding that shadowing is conceptually appropriate and am not having difficulty understanding. This is primarily from writing code as opposed to reading it though.

sam

Contributor

sammyd commented Nov 23, 2014

I often find myself shadowing the original variable, but I am in two minds as to whether this constitutes good style or not:

if let title = title {
   // do something with title
}

I like that we're not making up pointless new variable names (e.g. unwrappedTitle), however, I think there are times that shadowing can be confusing.

I am finding that shadowing is conceptually appropriate and am not having difficulty understanding. This is primarily from writing code as opposed to reading it though.

sam

@cwagdev

This comment has been minimized.

Show comment
Hide comment
@cwagdev

cwagdev Nov 23, 2014

I'm with Sam. Coming up with other names just seems starnge to me. Or having an instance variables like possibleTitle feels weird. 


Chris

On Sun, Nov 23, 2014 at 5:28 AM, Sam Davies notifications@github.com
wrote:

I often find myself shadowing the original variable, but I am in two minds as to whether this constitutes good style or not:

if let title = title {
   // do something with title
}

I like that we're not making up pointless new variable names (e.g. unwrappedTitle), however, I think there are times that shadowing can be confusing.
I am finding that shadowing is conceptually appropriate and am not having difficulty understanding. This is primarily from writing code as opposed to reading it though.

sam

Reply to this email directly or view it on GitHub:
#64 (comment)

cwagdev commented Nov 23, 2014

I'm with Sam. Coming up with other names just seems starnge to me. Or having an instance variables like possibleTitle feels weird. 


Chris

On Sun, Nov 23, 2014 at 5:28 AM, Sam Davies notifications@github.com
wrote:

I often find myself shadowing the original variable, but I am in two minds as to whether this constitutes good style or not:

if let title = title {
   // do something with title
}

I like that we're not making up pointless new variable names (e.g. unwrappedTitle), however, I think there are times that shadowing can be confusing.
I am finding that shadowing is conceptually appropriate and am not having difficulty understanding. This is primarily from writing code as opposed to reading it though.

sam

Reply to this email directly or view it on GitHub:
#64 (comment)

@ishuo

This comment has been minimized.

Show comment
Hide comment
@ishuo

ishuo Nov 23, 2014

Sounds like a practical solution. It could be confusing though, because one variable name maps to multiple types now.

Shuo

ishuo commented Nov 23, 2014

Sounds like a practical solution. It could be confusing though, because one variable name maps to multiple types now.

Shuo

@ColinEberhardt

This comment has been minimized.

Show comment
Hide comment
@ColinEberhardt

ColinEberhardt Nov 23, 2014

Member

Looking back at my Swift code, it's a bit of a mess regarding optional naming. I've used Sam's technique of 'shadowing' in a few places, but have also got some horrible names like maybeTitle, and unwrappedTitle - I've also seen one or two people use underscores as a prefix for unwrapped variables / constants.

All horrible!

Personally I think the shadowing approach is the most elegant.

Member

ColinEberhardt commented Nov 23, 2014

Looking back at my Swift code, it's a bit of a mess regarding optional naming. I've used Sam's technique of 'shadowing' in a few places, but have also got some horrible names like maybeTitle, and unwrappedTitle - I've also seen one or two people use underscores as a prefix for unwrapped variables / constants.

All horrible!

Personally I think the shadowing approach is the most elegant.

@icanzilb

This comment has been minimized.

Show comment
Hide comment
@icanzilb

icanzilb Nov 23, 2014

Member

I agree that shadowing is most elegant. However it, for me, never stopped feeling wrong

Member

icanzilb commented Nov 23, 2014

I agree that shadowing is most elegant. However it, for me, never stopped feeling wrong

@ishuo

This comment has been minimized.

Show comment
Hide comment
@ishuo

ishuo Nov 23, 2014

For me unwrappedTitle is the most clean way, just that the prefix "unwrapped" is too long. Something concise like uTitle yet still express the "unwrappedness" clearly would be awesome, isn't it

ishuo commented Nov 23, 2014

For me unwrappedTitle is the most clean way, just that the prefix "unwrapped" is too long. Something concise like uTitle yet still express the "unwrappedness" clearly would be awesome, isn't it

@scalessec

This comment has been minimized.

Show comment
Hide comment
@scalessec

scalessec Nov 24, 2014

I also think shadowing is the cleanest approach, but I too have concerns about legibility. What are your thoughts about naming conventions when dealing with collections of type AnyObject, as is the case when interfacing with Obj-C? Consider this:

APIClient.getObjects({(objects:[AnyObject]!) -> Void in
    if let objects = objects as? [Object] {

    }
})

I was originally prefixing "any" to collections of type AnyObject, but "anyObjects" just doesn't read/feel right. Think I'll stick to shadowing.

I also think shadowing is the cleanest approach, but I too have concerns about legibility. What are your thoughts about naming conventions when dealing with collections of type AnyObject, as is the case when interfacing with Obj-C? Consider this:

APIClient.getObjects({(objects:[AnyObject]!) -> Void in
    if let objects = objects as? [Object] {

    }
})

I was originally prefixing "any" to collections of type AnyObject, but "anyObjects" just doesn't read/feel right. Think I'll stick to shadowing.

@gregheo

This comment has been minimized.

Show comment
Hide comment
@gregheo

gregheo Dec 9, 2014

Member

I'm feeling against putting "unwrapped" or "u" in front as it starts to smell like much-maligned Hungarian notation, which strong typing should be saving us from.

I like the idea of shadowing. I haven't seen many optional variables named like "maybeTitle" but rather just "title" and I don't see the benefit of "maybeTitle" and "reallyTitle" or any such prefixes – just call it a title and whether it's a String? or if-let-bound as a String, you'll know it at compile time.

Member

gregheo commented Dec 9, 2014

I'm feeling against putting "unwrapped" or "u" in front as it starts to smell like much-maligned Hungarian notation, which strong typing should be saving us from.

I like the idea of shadowing. I haven't seen many optional variables named like "maybeTitle" but rather just "title" and I don't see the benefit of "maybeTitle" and "reallyTitle" or any such prefixes – just call it a title and whether it's a String? or if-let-bound as a String, you'll know it at compile time.

@ishuo

This comment has been minimized.

Show comment
Hide comment
@ishuo

ishuo Dec 9, 2014

Thanks for all the opinions! I'm now convinced of the shadowing approach. Should we expand the style guide to include this?

ishuo commented Dec 9, 2014

Thanks for all the opinions! I'm now convinced of the shadowing approach. Should we expand the style guide to include this?

@rwenderlich

This comment has been minimized.

Show comment
Hide comment
@rwenderlich

rwenderlich Dec 9, 2014

Member

👍 for shadowing - let's get this into the guide as it's a common question/problem we have.

Member

rwenderlich commented Dec 9, 2014

👍 for shadowing - let's get this into the guide as it's a common question/problem we have.

@gregheo

This comment has been minimized.

Show comment
Hide comment
@gregheo

gregheo Dec 11, 2014

Member

Thanks for starting the discussion here @ishuo!

Member

gregheo commented Dec 11, 2014

Thanks for starting the discussion here @ishuo!

gregheo added a commit that referenced this pull request Dec 11, 2014

Merge pull request #64 from gregheo/64-optional-binding
Variable name for Optional Binding

@gregheo gregheo merged commit 9fdd164 into raywenderlich:master Dec 11, 2014

@gregheo gregheo deleted the gregheo:64-optional-binding branch Dec 11, 2014

@patchin

This comment has been minimized.

Show comment
Hide comment
@patchin

patchin Feb 19, 2015

👎 I got here because I saw this shadowing being used in an NSHipster article w/o explanation and had to Google around to figure out what the real deal was. I won't be the only one. Feels hacky. Just to make it look nicer? Reminds me of the hubub concerning "optional" semicolons in javascript. Just because you can doesn't mean you should.

patchin commented Feb 19, 2015

👎 I got here because I saw this shadowing being used in an NSHipster article w/o explanation and had to Google around to figure out what the real deal was. I won't be the only one. Feels hacky. Just to make it look nicer? Reminds me of the hubub concerning "optional" semicolons in javascript. Just because you can doesn't mean you should.

@johngoren

This comment has been minimized.

Show comment
Hide comment
@johngoren

johngoren Feb 19, 2015

I sometimes go with let title = myTitle, but this sounds like an online product name from 2002, so I change it to the more generous and plummy let title = ourTitle.

I sometimes go with let title = myTitle, but this sounds like an online product name from 2002, so I change it to the more generous and plummy let title = ourTitle.

@ishuo

This comment has been minimized.

Show comment
Hide comment
@ishuo

ishuo Mar 2, 2015

@patchin I am now used to the shadowing. Works pretty well in my projects, pragmatic! Of course you can make suggestions if you have better ideas.

ishuo commented Mar 2, 2015

@patchin I am now used to the shadowing. Works pretty well in my projects, pragmatic! Of course you can make suggestions if you have better ideas.

@patchin

This comment has been minimized.

Show comment
Hide comment
@patchin

patchin Mar 19, 2015

How about:

if let _foo = foo {
}

or:

if let _foo_ = foo {
}

It also has the visual effect of looking unwrapped.

patchin commented Mar 19, 2015

How about:

if let _foo = foo {
}

or:

if let _foo_ = foo {
}

It also has the visual effect of looking unwrapped.

@RobertGummesson

This comment has been minimized.

Show comment
Hide comment
@RobertGummesson

RobertGummesson Mar 19, 2015

Contributor

Been following the various discussions on this topic and thought I'd add my view. To me, shadowing felt a bit strange at first just as not prefixing 'properties' with self did. Now I've gotten used to both and don't look back.

I agree 100% with what @ColinEberhardt wrote on issue #75.
"Hungarian Notation has almost died out, there's no good reason to revive it!"

Regarding using underscores as prefixes, I don't personally distinguish between private and public variables in Swift. ...not yet. If I would, I imagine they would look like this:
private var _foo

I would in other words expect underscore to have the same meaning in Swift as in most other languages I've worked with. So I'd be careful giving them a different meaning in Swift.

Contributor

RobertGummesson commented Mar 19, 2015

Been following the various discussions on this topic and thought I'd add my view. To me, shadowing felt a bit strange at first just as not prefixing 'properties' with self did. Now I've gotten used to both and don't look back.

I agree 100% with what @ColinEberhardt wrote on issue #75.
"Hungarian Notation has almost died out, there's no good reason to revive it!"

Regarding using underscores as prefixes, I don't personally distinguish between private and public variables in Swift. ...not yet. If I would, I imagine they would look like this:
private var _foo

I would in other words expect underscore to have the same meaning in Swift as in most other languages I've worked with. So I'd be careful giving them a different meaning in Swift.

@patchin

This comment has been minimized.

Show comment
Hide comment
@patchin

patchin Mar 19, 2015

  1. In this case it's not a private variable (private class member as I think you're trying to say), it's a local variable.
  2. Let's not call it Hungarian Notation because of all the bad connotations. I still think it's useful to be able to look at a line of code and divine what the types are without the assistance of the IDE. Especially when that code is published on a website, like GitHub.

patchin commented Mar 19, 2015

  1. In this case it's not a private variable (private class member as I think you're trying to say), it's a local variable.
  2. Let's not call it Hungarian Notation because of all the bad connotations. I still think it's useful to be able to look at a line of code and divine what the types are without the assistance of the IDE. Especially when that code is published on a website, like GitHub.
@RobertGummesson

This comment has been minimized.

Show comment
Hide comment
@RobertGummesson

RobertGummesson Mar 19, 2015

Contributor

@patchin - My point was that a local variable would then risk having the same prefix as a private class member. So far I haven't seen any guidelines for private vs. public class member prefixes but I would imagine that we'll get there one day even in Swift.

Contributor

RobertGummesson commented Mar 19, 2015

@patchin - My point was that a local variable would then risk having the same prefix as a private class member. So far I haven't seen any guidelines for private vs. public class member prefixes but I would imagine that we'll get there one day even in Swift.

@patchin

This comment has been minimized.

Show comment
Hide comment
@patchin

patchin Mar 19, 2015

I swear, you've edited your comment after I've made mine. Ok then, use the underscore as a suffix.

patchin commented Mar 19, 2015

I swear, you've edited your comment after I've made mine. Ok then, use the underscore as a suffix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment