RSS
 

Posts Tagged ‘Coding Standards’

Avoid Yoda conditions

15 Oct

After blogging about the semantic importance of void as pseudo type I feel like I should also point out the nonsense of Yoda conditions.
Those are still used quite a bit in the coding world, mainly in and around WordPress and Symfony if I recall correctly.

First of all, what is it?
It is the switching of order in conditions:

if (2 === $value) {
}

Like u say “If blue is the sky”.

Sounds weird? Sure is 🙂

Why do they exist?

It seems they have been invented to prevent "some" mistakes (some as in only a small subset!) when writing conditions.
If you, for some reason (…?), forget to write the second = in == it will result in a parse error for a few cases, telling you that quite early in the development process.

What is the danger of using them?

The main question about coding standards is always the same. Do we write clean and readable code for the computer or for humans.
The computer usually doesn’t care about any of those standard issues. The code functionally behaves the same.
But if humans have a hard time understanding/reading the code, that will be a huge problem for maintainability.
It also introduces a high risk of making errors.
So actually, in my many years of programming I have actually seen more mistakes made because of Yoda then without.

Think about this

if (true !== is_numeric($var)) {
}
// vs.
if (is_numeric($var) !== true)  {
}

or even

if (2 < $value) {
}
// vs.
if ($value > 2) {
}

You can easily forget to negate a value when in the not so natural order or confuse the comparison direction.

Often times I see them used only for the equality checks, not for the rest of the comparisons, and that makes it super inconsistent.

if (false === $foo) {
	...
} elseif ($bar > 2)  {
	...
}

Eliminating the root problem

It seems the Yoda fans have simply forgotten to look for the root issue they try to prevent here.
And instead of fixing that, they made a half-hearted workaround that misses out on most accidental assignments and introduces
the right of human error due to the mental overhead involved.

Actually, it gives you a false sense of security.
What is won if you still can by accident do

// Autsch
if ($variable = $otherVariable) {
    // ...
}

So let’s fix it – the right way.

Preventing inline assignment

Prevent what you want to "hack avoid" consistently and without exceptions:

// Conditional assignment is disallowed
if (($variable = $this->foo()) === null) {
    // ...
}
// This is how it's done
$variable = $this->foo();
if ($variable === null) {
    // ...

Add a sniffer to be sure

Now add a sniffer to your CS (php-cs-fixer or phpcs tool) and automatically prevent this from happening ever
(100% error free, cannot be missed by human error).
Make sure, you cannot merge until the sniffer shows a "green light".
That means = to check for only, as all other operations are not assignment operators.

And now we can add a second sniffer to check on no primitive value is the first argument: 'foo'|true|false|null|, numbers (1…x), constants and CO.

Setup your IDE

IDEs like PHPStorm can detect and warn at runtime about inline assignment:

Settings => Editor => Inspections => Probable bugs => Assignment in condition (check)

Benefit

The chance of assigning by accident went from likely to not possible.
In the meantime readability increased a lot, as the natural language makes this "natural" in the way we read and understand the conditions without having
to do further calculations in our head.

Further reading

blog.codinghorror.com/new-programming-jargon says it all.
This is also valued by some of the most important PHP open source frameworks, like CakePHP.

It is also part of PSR-2-R Additions.

Update 2015-12

I stumbled upon an article where they also relized the same thing.

 
3 Comments

Posted in PHP

 

Coding Standards in PHP

16 Jan

I feel about this very strongly. At least about some particular topics.
It is very important for every group project (even for a one man project – you never know when sobody might join you) to have some common guidelines how to write code.
Some projects don’t seem to have them at all. Others don’t care.
And I often see the weirdest combinations of standards – even in some bigger projects.

Quite a while ago I already published a complete summary (see the link below).
This time I want to go into details what PHP is concerned.

I will only cover the main aspects – the most important standards.
The full list can be found here.

It’s the single most annoying thing to clean up somebody else’s code which is messed up with the wrong LFs, indent signs, spaces etc.
So I published guidelines that make the most sense for my projects.

Indentation

Some projects use spaces. Absolutely wrong. Spaces are not supposed to be used this way. You can use them to separate sentences, even conditions like ($x && $y). But to indent code there is only one correct solution called "tabs". One tab for each level of indentation.

The nice side-effect: A good IDE can transform this into pseudo-spaces, allowing the user to decide how much width a single tab has. Some like more, some less indentation length per level.
Either way the code is easily transportable/editable cross-browser and cross-operationsystem.

Tip: This can easily be applied globally using my IndentShell

Curly Braces

This can really be a pain in the … sometimes. As with most of the important php projects they MUST be opened in the same line. It increases readability (less lines on the monitor) without a single disadvantage.
Therefore I can’t and I won’t understand why somebody would do it otherwise.

if (condition1 || condition2) {
	action1;
} elseif (condition3 && (condition4 || condition5)) {
	action2;
} else {
	defaultaction;
}

Additionally, they must also be used in situations where they are technically optional (due to the inline format they only add a single line of code, but again increase readability + make it easy to add code).

Spaces

After commas one space, before curly opening brackets one space and between operaters one space:

# wrong (6 spaces missing)
if(($var1)||($var2)){
	$res=array_merge($defaultOptions,$options);
}
# correct
if (($var1) || ($var2)) {
	$res = array_merge($defaultOptions, $options);
}

if/else/elseif

Those should always include {} – even if is a one-liner. And elseif should be used instead of else if.
@see are-elseif-and-else-if-completely-synonymous for a discussion on that.

Comments

Some discourage the usage of the shell style comment sign (#). I don’t think this is justified.
A lot of code – especially in developing process – has commented out code fragments. At some point you don’t even know anymore what is temporary commented out and what is a real comment.

# apply style (REAL COMMENT)
$this->apply($data);
/*
$this->isCommentedOutForSomeReason();
*/
...
// blabla
//$this->add('oh, this too, is commented out!');
...
# Another important comment here!?
$this->Model->setRecursive(1);
...

So if a programmer uses // for single lines and /* */ for multiple lines to comment them out, he can easily use # for actual comments. This way real comments are distinguishable from commented out stuff more easily.

Closing Tags in PHP

Files that contain only PHP code should not include the closing tag ?> as it can lead to header problems with the HTTP response if the text editor you’re using automatically adds a new line to the end of your files.

Tip: Those can easily be removed using my PhpTagShell

Automation

If you want to automate CS checks, use phpcs. There are CakePHP sniff rules for the core. I use my own codesniffer with a few more features available – and to comply with my coding standards that are built on top of the core ones.

Vista

If everyone used the same coding standards (especially tabs for indent, inline spaces and some other common guidelines) we wouldn’t have any trouble adapting foreign code or code by other programmers into the own repository.
Nobody would ever write HTML like <sPaN cLaSs='foo'>...</SPan> – so why not trying to establish some common ground rules on PHP?

 
1 Comment

Posted in PHP