4 ms·

Some of this seems petty. For example, the author gives the following "bad" example: def method (object) if object.property param = object.propert

Some of this seems petty. For example, the author gives the following "bad" example: def method (object) if object.property param = object.property else param = default_param end end And suggests that this is better: def method (object) param = object.property || default_param end To me, this is an example of an if statement by another name. Sure, you cut out a few lines, but it's still an if-then construct. Both examples are most likely going to be treated the same way by the compiler as well.

Adirael 14y ago
I agree, and I prefer the first one, it just seems clearer for me.
michaelfeathers 14y ago
I disagree. Sure, the code is functionally equivalent today, but the next person touching the 'or' version might be less likely to introduce side-effecting computation. The 'if-else' version is completely open-ended. We need to think about the effect our choices have on the next person who touches the code. Often with a different baseline people make different changes. It's a micro-form of 'No Broken Windows'.
- delinka 14y ago
  "We need to think about the effect our choices have on the next person who touches the code." And that next person might be the newbie on the team that doesn't have the business domain knowledge built up over years of working for This Company. So I'll use the if statement to keep intent clear. Or maybe it'll be someone whose primary programming language doesn't use the || operator. Or maybe, for some unknown reason, it'll be a PHB. So I'll use the if statement to keep intent clear.
  - michaelfeathers 14y ago
    Setting the baseline gives you a higher probability of a better change than you would have otherwise. This assumes that not everyone on the team is a newb or a code-dabbling PHB. If they are, there's not much point in writing any code.
  - mattvanhorn 14y ago
    If the idea is to keep the intent clear, the or statement is a much better choice, even if less familiar. "I want to set param to the object property or the default" is much clearer than "I want to check if if I have an object property and if I do then I'll set the param to the object property otherwise I'll set the param to the default" That is just mixing in a lot of noise about how you do it instead of just what you are doing.
manojlds 14y ago
I agree. The null coalescing is just a syntax sugar for if. While it should be used, it is not the right example in this context.
gizzlon 14y ago
Absolutely! And in the second example, what happens if "property" is 0 ? (My Ruby is rusty)
- paradoja 14y ago
  0 evaluates to true in Ruby. Only nil and false evaluate to false in Ruby, so it'd work as intended.
alisnic 14y ago
I was talking in terms of the constructs which the language can provide to avoid ifs. > And suggests that this is better: It depends in what terms are you thinking. A syntactic improvement is a also a benefit, isn't it?
- tallanvor 14y ago
  Whether or not the second example represents a syntactic improvement is more of a personal preference. Personally, I find the first example more readable when quickly scanning code.
  - jbrains 14y ago
    Yes, but that's probably a matter of familiarity over clarity. This is a perfect time to use the irrationally-dreaded ternary operator. param = object.property.nil? ? default_value : object.property Verbose, although less so, but reveals intent almost perfectly. Would be better if the colon were "else" or "otherwise".
jbrains 14y ago
Not an if statement, but unnecessarily clever code that doesn't reveal its intent. An example of the tension between general clarity and a concise, well-known idiom. The kind of thing that makes C++ impenetrable to those who don't write it regularly. I'd rather add a method to Object/Nil called something like #or_else so that we could write param = object.property.or_else(default_param) which, though still terse, at least describes what's going on.
jbrains 14y ago
I just realised that the statement "it's still an if-then construct" illustrates a common problem among people practising OOP: conflating implementation with interface. The OR is not an if-then construct. If anything, the if-then construct is a specialised use of OR, given than if-then-else is literally XOR. Both are an implementation of the interface "coalesce a value with null/0/false". I only mention this because it points to a lack of precision in our thinking which I see time after time get in the way of using OOP/OOD effectively. Maybe that's a failing of OOP, but I think that if it were, it'd be a failing of programming in general, too. This helps explain why I teach OOP the way I do: start by following the rules of removing duplication and improving names, which encourages the programmer to ask ever-more-interesting questions about what tools are available to help follow those rules, which encourages the programmer to learn OO theory as needed, rather than having it shoved in their face all at once. This just-in-time learning leads to longer-lasting understanding for many (most?) people.
codehalo 14y ago
I don't like the OP code either. To someone not familiar with this language, || produces a boolean value (true or false), and that value gets assigned to param. A better approach would be (without knowledge of the language in particular): def method (object) param = default_param if object.property param = object.property end end Here, its clear that param has a default right away.

Adirael 14y ago

I agree, and I prefer the first one, it just seems clearer for me.

michaelfeathers 14y ago

I disagree. Sure, the code is functionally equivalent today, but the next person touching the 'or' version might be less likely to introduce side-effecting computation. The 'if-else' version is completely open-ended. We need to think about the effect our choices have on the next person who touches the code. Often with a different baseline people make different changes. It's a micro-form of 'No Broken Windows'.

delinka 14y ago
"We need to think about the effect our choices have on the next person who touches the code." And that next person might be the newbie on the team that doesn't have the business domain knowledge built up over years of working for This Company. So I'll use the if statement to keep intent clear. Or maybe it'll be someone whose primary programming language doesn't use the || operator. Or maybe, for some unknown reason, it'll be a PHB. So I'll use the if statement to keep intent clear.
- michaelfeathers 14y ago
  Setting the baseline gives you a higher probability of a better change than you would have otherwise. This assumes that not everyone on the team is a newb or a code-dabbling PHB. If they are, there's not much point in writing any code.
- mattvanhorn 14y ago
  If the idea is to keep the intent clear, the or statement is a much better choice, even if less familiar. "I want to set param to the object property or the default" is much clearer than "I want to check if if I have an object property and if I do then I'll set the param to the object property otherwise I'll set the param to the default" That is just mixing in a lot of noise about how you do it instead of just what you are doing.

manojlds 14y ago

I agree. The null coalescing is just a syntax sugar for if. While it should be used, it is not the right example in this context.

gizzlon 14y ago

Absolutely! And in the second example, what happens if "property" is 0 ? (My Ruby is rusty)

paradoja 14y ago
0 evaluates to true in Ruby. Only nil and false evaluate to false in Ruby, so it'd work as intended.

alisnic 14y ago

I was talking in terms of the constructs which the language can provide to avoid ifs. > And suggests that this is better: It depends in what terms are you thinking. A syntactic improvement is a also a benefit, isn't it?

tallanvor 14y ago
Whether or not the second example represents a syntactic improvement is more of a personal preference. Personally, I find the first example more readable when quickly scanning code.
- jbrains 14y ago
  Yes, but that's probably a matter of familiarity over clarity. This is a perfect time to use the irrationally-dreaded ternary operator. param = object.property.nil? ? default_value : object.property Verbose, although less so, but reveals intent almost perfectly. Would be better if the colon were "else" or "otherwise".

jbrains 14y ago

Not an if statement, but unnecessarily clever code that doesn't reveal its intent. An example of the tension between general clarity and a concise, well-known idiom. The kind of thing that makes C++ impenetrable to those who don't write it regularly. I'd rather add a method to Object/Nil called something like #or_else so that we could write param = object.property.or_else(default_param) which, though still terse, at least describes what's going on.

jbrains 14y ago

I just realised that the statement "it's still an if-then construct" illustrates a common problem among people practising OOP: conflating implementation with interface. The OR is not an if-then construct. If anything, the if-then construct is a specialised use of OR, given than if-then-else is literally XOR. Both are an implementation of the interface "coalesce a value with null/0/false". I only mention this because it points to a lack of precision in our thinking which I see time after time get in the way of using OOP/OOD effectively. Maybe that's a failing of OOP, but I think that if it were, it'd be a failing of programming in general, too. This helps explain why I teach OOP the way I do: start by following the rules of removing duplication and improving names, which encourages the programmer to ask ever-more-interesting questions about what tools are available to help follow those rules, which encourages the programmer to learn OO theory as needed, rather than having it shoved in their face all at once. This just-in-time learning leads to longer-lasting understanding for many (most?) people.

codehalo 14y ago

I don't like the OP code either. To someone not familiar with this language, || produces a boolean value (true or false), and that value gets assigned to param. A better approach would be (without knowledge of the language in particular): def method (object) param = default_param if object.property param = object.property end end Here, its clear that param has a default right away.