log-oscon / Standards

Coding standards and whatnot.
https://log-oscon.github.io/Standards/
MIT License
7 stars 0 forks source link

Line breaks when there's only one item between braces #17

Closed lenonleite closed 7 years ago

lenonleite commented 7 years ago
if ( isset ( $_GET['intermediary_id'] ) && ! empty( $_GET['intermediary_id'] ) ) {
   $this->details( $_GET['intermediary_id'], $this->campaign_id );
}
 if ( isset ( $_GET['intermediary_id'] ) && ! empty( $_GET['intermediary_id'] ) ) {

    $this->details( $_GET['intermediary_id'], $this->campaign_id );

 }
 if ( $key !== $this->campaign_id ) {

    $this->details( $this->intermediary_id, $key );
 }
liciniofs commented 7 years ago

The first example is easier to identify and read.

lenonleite commented 7 years ago

For me, the more interesting and that have more sense is example 2 because align and is more clean. I am start of propose that if you broke line in init you have broke in finish.

Narayon commented 7 years ago

For me, breaking at the end doesn't make sense.
Moreover, if you have a simple example, like this:

if ( is_wp_error( $client ) ) {
    return;
}

It doesn't make sense to break at the beginning either.
I think you should only break at the beginning only when the code after is long and might affect the legibility of the cycle definition.

lenonleite commented 7 years ago

But observe, if have two single two options you broke and one item no broke. Is this no make sense.

if ( is_wp_error( $client ) ) {     return; }

ok but

if ( is_wp_error( $client ) ) {          $var = 'a';          return $var;      }

not ok? for me have similar ideia.

Narayon commented 7 years ago

This discusion and standard are all about improving legibility.
Adding a line break at the end doesn't make sense because it doesn't improve the legibility. In my opinion, it decreases because you're isolating the logic too much.
When the logic is multiline, you can add a line break at the top, if you think that line will become easier to read.
Besides that, return statements should always be prefixed with a line break, when the logic has more than one line or the return expression(a long one) is decreasing the legibility of the cycle definition.

lenonleite commented 7 years ago

sorry, closed issue unintentionally

liciniofs commented 7 years ago

@lenonleite They are two different cases.

In the second one it's not a single line block, so, it makes sense to have the line breaks as you did.

csrui commented 7 years ago

I'm all for example 3. Why?

I do no agree with @liciniofs on the Example 1 being easier to read. At all... What it does is help show you more code at the same time like @Narayon states.

According to Design / Usability principles whitespace improves readability therefor having a bunch of lines glued together would only make it harder to read.

Then why not Example 2? No point on having a line break before the } closing of something.

Regarding the return I think I agree with @Narayon on the return statements being prefixed with a line break. It helps identify where a method steps out.

liciniofs commented 7 years ago

@csrui I didn't explain myself the right way. When I wrote easier to read, I mean easier to identify when looking for patterns in the code. I find it harder to recognise blocks if they're not a block.

RicCastelhano commented 7 years ago

In this one I'm all for the option#3. Like @csrui said, white space helps readability (but not too much whitespace). When a method have a return, I'm all for the extra blank line before de return execution (the reasons are the same as @csrui stated).