kodecocodes / objective-c-style-guide

A style guide that outlines the coding conventions for Kodeco
http://www.raywenderlich.com/62570/objective-c-style-guide
3.1k stars 628 forks source link

Be explicit about the datatype when comparing #22

Closed hollance closed 10 years ago

hollance commented 10 years ago

I have this open as a pull request but it's a bit hidden, so I'm opening up a new issue for this as well.

In the guide it currently says this is how you check for nil:

if (someObject) ...
if (!someObject)...

I strongly suggest we are always explicit about this sort of thing:

if (someObject != nil) ...
if (someObject == nil)...

That makes it immediately clear what you're comparing.

I'd like the syntax if (a) and if (!a) to be limited to booleans only.

So not this either:

if (someInt) { ... }

The only exception I make for this in my own code is init methods:

if ((self = [super init]))

because that is idiomatic.

icanzilb commented 10 years ago

I really prefer this myself, because it's just so clear to read:

NSString* str = [obj stringForblablabla];
if (str) {
  //do something with the string
}
funkyboy commented 10 years ago

For brevity I agree, but for learning and readability I prefer

if (str == nil)

On Fri, Nov 8, 2013 at 10:50 AM, Marin Todorov notifications@github.comwrote:

I really prefer this myself, because it's just so clear to read:

NSString* str = [obj stringForblablabla];if (str) { //do something with the string}

— Reply to this email directly or view it on GitHubhttps://github.com/raywenderlich/objective-c-style-guide/issues/22#issuecomment-28051637 .

Cesare Rocchi http://studiomagnolia.com

moayes commented 10 years ago

Agree that it makes it easier to read but ... it is a language feature, you don't have to compare against nil, nor you have to compare against 0.

pietrorea commented 10 years ago

I wouldn't write 'if (str == nil)' myself because it leaves me open to if (str = nil). But for someone who is new to Objective-C (or programming in general) this tiny bit of verbosity would be very helpful.

hollance commented 10 years ago

@moayes Yes, it's a language feature. A potentially confusing one.

Just because something is a language feature doesn't mean it should always be accepted. Some language features just aren't any good.

ghost commented 10 years ago

I disagree. If(obj) is a standard way to check for non-nil values. Not showing that on the site isn't helping people learn anything, it is opening them up to not understanding any real code ever, because no one mentions nil in a check for nil.

I agree that checking ints should be explicit, but not objects.

Sent from my iPhone

On Nov 8, 2013, at 12:35 PM, Matthijs Hollemans notifications@github.com wrote:

@moayes Yes, it's a language feature. A potentially confusing one.

Just because something is a language feature doesn't mean it should always be accepted. Some language features just aren't any good.

— Reply to this email directly or view it on GitHub.

moayes commented 10 years ago

Maybe for a beginner level tutorial it is better to say if (object != nil) but for an intermediate and above, I think it is better to say if (object).

icanzilb commented 10 years ago

@pietro - xcode produces a warning for if (str = nil)

icanzilb commented 10 years ago

@moayes I think that's unnecessary complication for the authors to change their style depending on the tutorial level ...

Did I say I love if (myObjec) { do stuff } ? It's the best

gregheo commented 10 years ago

I don't like checking for nil explicitly. Next thing, you'll be saying if ((str == nil) == YES)!

Just kidding! Seriously though, I think checking for non-nil pointers (or non-NULL pointers going back to C) with if (!thing) is pretty idiomatic.

funkyboy commented 10 years ago

if (!str) is pretty clear if(str) is not immediately intuitive.

I am fine either way, but I still think str == nil enhances readability. To support my claim: Big Nerd Ranch teachers suggest the == nil, at least in the beginning.

I don't like checking for nil explicitly. Next thing, you'll be saying `if ((str == nil) == YES)'!

Just kidding! Seriously though, I think checking for non-nil pointers (or non-NULL pointers going back to C) with if (!thing) is pretty idiomatic.

— Reply to this email directly or view it on GitHub.

ghost commented 10 years ago

It doesn't need to be immediately intuitive the first time you see it. The COBOL language was very intuitive, but you wouldn't want to write anything in it today.

Adding == nil is simply something you should never expect to see. If our site is coding like that, anyone who knows how to code will assume we don't.

Sent from my iPhone

On Nov 8, 2013, at 2:20 PM, Cesare notifications@github.com wrote:

if (!str) is pretty clear if(str) is not immediately intuitive.

I am fine either way, but I still think str == nil enhances readability. To support my claim: Big Nerd Ranch teachers suggest the == nil, at least in the beginning.

I don't like checking for nil explicitly. Next thing, you'll be saying `if ((str == nil) == YES)'!

Just kidding! Seriously though, I think checking for non-nil pointers (or non-NULL pointers going back to C) with if (!thing) is pretty idiomatic.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

icanzilb commented 10 years ago

To be honest, I do exactly the opposite. I always do:

if (str) {...};
if (str == nil) {...};

go figure ...

funkyboy commented 10 years ago

The point is that we are probably targeting people who do not know how to code in Obj-c or they are just getting started.

On Nov 8, 2013, at 8:25 PM, elephantronic notifications@github.com wrote:

It doesn't need to be immediately intuitive the first time you see it. The COBOL language was very intuitive, but you wouldn't want to write anything in it today.

Adding == nil is simply something you should never expect to see. If our site is coding like that, anyone who knows how to code will assume we don't.

Sent from my iPhone

On Nov 8, 2013, at 2:20 PM, Cesare notifications@github.com wrote:

if (!str) is pretty clear if(str) is not immediately intuitive.

I am fine either way, but I still think str == nil enhances readability. To support my claim: Big Nerd Ranch teachers suggest the == nil, at least in the beginning.

I don't like checking for nil explicitly. Next thing, you'll be saying `if ((str == nil) == YES)'!

Just kidding! Seriously though, I think checking for non-nil pointers (or non-NULL pointers going back to C) with if (!thing) is pretty idiomatic.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub. — Reply to this email directly or view it on GitHub.

pietrorea commented 10 years ago

@elephantronic readers want to get through the tutorials as quickly as possible. if our readers have to do a double take of any of the code that ships with our tutorials and books, we failed them.

i would rather write code that is 100% intuitive and unambiguous and have people think i don't know how to code than the other way around.

ghost commented 10 years ago

That's crazy talk. A double take means we've failed our readers? You guys all seem to think that our readers are in a semi-vegetative state or something. Give people some credit. You understand that code, right? Did you invent not being stupid? No? So maybe other people are capable of understanding this stuff, too.

if(obj) or if(!obj) IS the style we should be teaching because that's the style that people use in real life. We need to stop trying to invent our own style and teach the most commonly used practices, because people just may want to understand how to read code that isn't from this website, too.

Sent from my iPhone

On Nov 8, 2013, at 2:37 PM, Pietro Rea notifications@github.com wrote:

@elephantronic readers want to get through the tutorials as quickly as possible. if our readers have to do a double take of any of the code that ships with our tutorials and books, we failed them.

i would rather write code that is 100% intuitive and unambiguous and have people think i don't know how to code than the other way around.

— Reply to this email directly or view it on GitHub.

hollance commented 10 years ago

Yes, if (obj) and if (!obj) are used in real projects. But so are if (obj != nil) and if (obj == nil). Personally I think the former are a defect in the language (one of the few things Java does better). But I must concede that its usage is idiomatic indeed.

But right now the guide says if (obj == nil) is bad, which I disagree with.

ndubbs commented 10 years ago

The thing to remember with this topic is the end result, what do we want the code to look like regardless of skill level. We have a broad range of readers from beginner to advanced. All the coding in our tutorials SHOULD be consistent and that is the reason for doing this style guide. It is the job of the tutorial writer to explain why the code is written in the manner it is.

@hollance We are going to change the word choices of good and bad. That should take care of the negative connotation that (obj == nil) is bad.

Everyone, let's have a vote on which way we prefer. Please reply with your choice. I think the reasons why have been covered. :)

pietrorea commented 10 years ago

(obj == nil)

funkyboy commented 10 years ago

(obj == nil)

Cesare Rocchi studiomagnolia.com

On Nov 9, 2013, at 4:23 PM, Pietro Rea notifications@github.com wrote:

(obj == nil)

— Reply to this email directly or view it on GitHub.

ghost commented 10 years ago

if(obj)

Sent from my iPhone

On Nov 9, 2013, at 11:40 AM, Cesare notifications@github.com wrote:

(obj == nil)

Cesare Rocchi studiomagnolia.com

On Nov 9, 2013, at 4:23 PM, Pietro Rea notifications@github.com wrote:

(obj == nil)

— Reply to this email directly or view it on GitHub. — Reply to this email directly or view it on GitHub.

moayes commented 10 years ago

+1 if (obj)

icanzilb commented 10 years ago

yet again:

if (obj) ... 
if (obj == nil) ... 

this way you have only positive comparisons

hollance commented 10 years ago

Since you asked:

if (obj != nil)
if (obj == nil)

I don't think a vote on what everyone prefers is the same as finding the best way to teach programming, though. ;-)

mattjgalloway commented 10 years ago
if (obj)
if (!obj)

... for me. It's just cleaner, obvious and perfectly valid.

I do however use, and prefer this:

if (myArray.count > 0)

... because I think that it's better to be explicit about the fact you're checking that there are more than zero objects in that array.

rwenderlich commented 10 years ago

My vote (but I'd be happy with whatever we decide):

if (obj)
if (!obj)

Because I think this is used more commonly in real life.

Perhaps this is another good one for the "suggested but optional" section ;]

gregheo commented 10 years ago

+1 to checking bools and for nil or NULL: if (foo) and if (!foo)

But I agree with others that checking for numeric zero should be explicit: if (fooCount > 0)

ricardo-rendoncepeda commented 10 years ago

+1 for

if(obj)
if(!obj)

I think it reads better for development tutorials when you are trying to make sense of new logic (if object, if not object - rather than using any double-negatives like "if object is not equal to nil")

ndubbs commented 10 years ago

The majority wins on this issue.

if (obj)
if (!obj)