wp-bootstrap / wp-bootstrap-navwalker

A custom WordPress nav walker class to fully implement the Twitter Bootstrap 4.0+ navigation style (v3-branch available for Bootstrap 3) in a custom theme using the WordPress built in menu manager.
https://wp-bootstrap.github.io/wp-bootstrap-navwalker/
GNU General Public License v3.0
3.37k stars 1.94k forks source link

Remove elseif for better readability and debugging #518

Closed cryptexvinci closed 3 years ago

cryptexvinci commented 3 years ago

https://softwareengineering.stackexchange.com/questions/206816/clarification-of-avoid-if-else-advice

Fixes #

Changes proposed in this Pull Request:

Testing instructions:

*

Proposed changelog entry for your changes:

IanDelMar commented 3 years ago

@cryptexvinci Thanks for taking the time to contribute to WP Bootstrap Navwalker! I, personally, do neither see better readability nor better debugging. What your changes imply is that users are no longer restricted to one icon font but can use multiple icons from different fonts per nav item. I think hardly any user would use this feature. It also implies that user can mix the linkmod classes which does not make sense and I'm not sure whether this will have adverse effects on layout. It further implies that users may mix linkmod classes and icon classes in a way that makes no sense. It also decreases performance by always checking each if statement even if another if statement has already evaluated to true. Is that what you intended to do?

EDIT: What usually is meant with statements like "avoid if-else" (see the link you provided) is to drop unnecessary else statements. Here a simple example:

function my_example_one( $a = 0 ) {
    if ( 0 === (int) $a ) {
        $b = 0;
    } else {
        $b = $a + 1;
    }
    return $b;
}

function my_example_one_wo_else( $a = 0 ) {
    $b = 0
    if ( 0 !== (int) $a ) {
        $b = $a + 1;
    }
    return $b;
}

// or:

function my_example_two( $a = 0 ) {
    if ( 0 === (int) $a ) {
        return;
    } else {
        return $a;
    }
}

function my_example_two_wo_else( $a = 0 ) {
    if ( 0 === (int) $a ) {
        return;
    }
    return $a;
}
IanDelMar commented 3 years ago

I'm closing this because of the probably unintended side effects mentioned above. If you still feel that your PR should be considered to be merged, we're open for a discussion about it.

I want to draw your attention to an answer provided at the link you posted: https://softwareengineering.stackexchange.com/a/206867