Empty ELSE statement in dbDelta()

[krunal265]

In the wp-admin/includes/upgrade.php there is an empty else statement.
Line: 2740

[hellofromTonya]

This empty else has been in the source code at least since WP 1.5. Resetting the Version to 1.5 though could have been early.

[SergeyBiryukov]

Hi there, welcome to WordPress Trac! Thanks for the ticket.

Just to save some time for anyone looking at the ticket, this is what the code in question​ looks like:

// Create a tablename index for an array ($cqueries) of queries.
foreach ( $queries as $qry ) {
	if ( preg_match( '|CREATE TABLE ([^ ]*)|', $qry, $matches ) ) {
		$cqueries[ trim( $matches[1], '`' ) ] = $qry;
		$for_update[ $matches[1] ]            = 'Created table ' . $matches[1];
	} elseif ( preg_match( '|CREATE DATABASE ([^ ]*)|', $qry, $matches ) ) {
		...
	} else {
		// Unrecognized query type.
	}
}

Introduced in [1575]. I believe the purpose of the empty else statement is to provide a comment to clarify the case when the query type is not recognized. It adds some context, though does not seem to affect anything further in the function. Does anyone have a strong opinion on whether it should be kept or removed?

[brookedot]

Does anyone have a strong opinion on whether it should be kept or removed?

My two cents either add an error/log message with something like wp_error or remove it. I don't find the comment particularly helpful and we could easily rework the code comments to not require the empty else

[costdev]

I don't have strong feelings, although I do wonder whether the comment adds anything beneficial to the reader's understanding of the code.

We could change the initial comment:

Create a tablename index for an array ($cqueries) of queries.

to

Create a tablename index for an array ($cqueries) of recognised query types.

In addition, if we're going to remove the comment and else, we could also use continue to help separate each case and help a little with readability by not having a wall of if/elseif.

---

Current

<?php
// Create a tablename index for an array ($cqueries) of queries.
foreach ( $queries as $qry ) {
        if ( preg_match( '|CREATE TABLE ([^ ]*)|', $qry, $matches ) ) {
                $cqueries[ trim( $matches[1], '`' ) ] = $qry;
                $for_update[ $matches[1] ]            = 'Created table ' . $matches[1];
        } elseif ( preg_match( '|CREATE DATABASE ([^ ]*)|', $qry, $matches ) ) {
                array_unshift( $cqueries, $qry );
        } elseif ( preg_match( '|INSERT INTO ([^ ]*)|', $qry, $matches ) ) {
                $iqueries[] = $qry;
        } elseif ( preg_match( '|UPDATE ([^ ]*)|', $qry, $matches ) ) {
                $iqueries[] = $qry;
        } else {
                // Unrecognized query type.
        }
}

Proposed

<?php
// Create a tablename index for an array ($cqueries) of recognised query types.
foreach ( $queries as $qry ) {
        if ( preg_match( '|CREATE TABLE ([^ ]*)|', $qry, $matches ) ) {
                $cqueries[ trim( $matches[1], '`' ) ] = $qry;
                $for_update[ $matches[1] ]            = 'Created table ' . $matches[1];
                continue;
        }

        if ( preg_match( '|CREATE DATABASE ([^ ]*)|', $qry, $matches ) ) {
                array_unshift( $cqueries, $qry );
                continue;
        }

        if ( preg_match( '|INSERT INTO ([^ ]*)|', $qry, $matches ) ) {
                $iqueries[] = $qry;
                continue;
        }

        if ( preg_match( '|UPDATE ([^ ]*)|', $qry, $matches ) ) {
                $iqueries[] = $qry;
        }
}

---

Thoughts?

[SergeyBiryukov]

Replying to costdev:

In addition, if we're going to remove the comment and else, we could also use continue to help separate each case and help a little with readability by not having a wall of if/elseif.

Yeah, I agree that looks better. Thanks!

[SergeyBiryukov]

In 55688:

Coding Standards: Remove an empty else statement in dbDelta().

Use continue to help separate each case for better readability, instead of having a wall of if/elseif.

Includes simplifying a similar fragment in make_site_theme_from_default().

Follow-up to [1575], [2037], [2040], [2044], [2346], [7999], [14080], [14485].

Props costdev, krunal265, hellofromTonya, brookedot, SergeyBiryukov.
Fixes #56982.

[SergeyBiryukov]

In 55698:

Coding Standards: Break out of the inner loop in make_site_theme_from_default().

This more closely matches the previous behavior with multiple if/elseif statements.

Follow-up to [55688].

See #56982.