log-oscon / Standards

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

Validations with more than one condition ( if/else ) #19

Open lenonleite opened 7 years ago

lenonleite commented 7 years ago

Example - 1

if ( empty( $intermediary_id ) || ! is_numeric( $intermediary_id ) || empty( $campaign_id ) || ! is_numeric( $campaign_id ) ) {

}

Example - 2

if ( empty( $intermediary_id )     || ! is_numeric( $intermediary_id )     || empty( $campaign_id )     || ! is_numeric( $campaign_id ) {

}

Example - 3

if (     $x === 3     || $x === 4     || $x === 4 ) {

}

lenonleite commented 7 years ago

I prefer last option for visuality you see that start and finish validation with ( ) and spaces. Is more easy for ready.

liciniofs commented 7 years ago

Example 3. Better pattern identification

Narayon commented 7 years ago

First of all, if you have multiple conditions that can be logically grouped, use a function for that.
This:

if ( empty( $intermediary_id )
|| ! is_numeric( $intermediary_id )
|| empty( $campaign_id )
|| ! is_numeric( $campaign_id ) ) {

}

becomes this:

if ( is_valid_id( $intermediary_id ) || is_valid_id( $campaign_id ) ) {

}

You should always strive to have small conditions, in number and length.
When that's not possible, I think the first option is more legible, but not very pretty.

csrui commented 7 years ago

This should be our bible but that's just me: http://www.php-fig.org/psr/psr-2/

Here's a smiley face 😃

Narayon commented 7 years ago

PSR-2 is not a perfect fit with the WP standards, but it matches the third example of this issue.
This should always be the starting point of our standards: https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/

RicCastelhano commented 7 years ago

@Narayon said it well. We must always have WordPress Standards as a starting point (mandatory to keep in the road to VIP'hood), however I think we should (and will) improve them with our vision on this subject.

RicCastelhano commented 7 years ago

Concerning the issue in question, I'm very used to the option#2 but I can see benefits with the option#3.

Narayon commented 7 years ago

Comming back to this question, I've seen another approach, in the source code of some popular libraries, which is:

if (
  first-condition
  &&
  second-condition
) {
  body
}

I think this is the best approach, since it separates the conditions from the operators, for better legibility.
That said, this is for exceptional cases. Always try to define a method that encapsulates the condition with a semantic name, like I described in my first comment.

RicCastelhano commented 6 years ago

@Narayon that approach seems great. It keeps readability high. However we should avoid complex logical conditions. Please, write a PR for this.