Improve code-quality/readability in the main file #422
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aristath
commented
Dec 13, 2023
Comment on lines
-351
to
+361
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Using direct query for interacting with custom database, safe variable used for table name, caching not required for one time operation. | ||
| $rule_count = $wpdb->get_var( $wpdb->prepare( 'SELECT count(*) FROM %i where rule = %s and siteid = %d and postid = %d and ignre = %d', $table_name, $rule['slug'], $siteid, $postid, 0 ) ); | ||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Using direct query for interacting with custom database, safe variable used for table name, caching not required for one time operation. | ||
| $rule_count = $wpdb->get_var( | ||
| $wpdb->prepare( | ||
| 'SELECT count(*) FROM %i where rule = %s and siteid = %d and postid = %d and ignre = %d', | ||
| $table_name, | ||
| $rule['slug'], | ||
| $siteid, | ||
| $postid, | ||
| 0 | ||
| ) | ||
| ); |
There was a problem hiding this comment.
The only change in queries was to split them to multiple lines so that it's easier to visually figure out the nesting levels and what the prepare does with the placeholders.
aristath
commented
Dec 13, 2023
Comment on lines
-363
to
+381
| $summary['errors'] = intval( $wpdb->get_var( $wpdb->prepare( 'SELECT count(*) FROM %i where siteid = %d and postid = %d and ruletype = %s and ignre = %d', $table_name, get_current_blog_id(), $post_id, 'error', 0 ) ) ); | ||
| $summary['errors'] = (int) $wpdb->get_var( | ||
| $wpdb->prepare( | ||
| 'SELECT count(*) FROM %i where siteid = %d and postid = %d and ruletype = %s and ignre = %d', | ||
| $table_name, | ||
| get_current_blog_id(), | ||
| $post_id, | ||
| 'error', | ||
| 0 | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Split this one (and others below) to multiple lines, but also removed the intval() call, replacing it with an (int) typecase which is faster.
aristath
commented
Dec 13, 2023
Comment on lines
-424
to
+463
| $text_statistics = new DaveChild\TextStatistics\TextStatistics(); | ||
| $summary['content_grade'] = floor( $text_statistics->fleschKincaidGradeLevel( $content ) ); | ||
| $summary['content_grade'] = floor( | ||
| ( new DaveChild\TextStatistics\TextStatistics() )->fleschKincaidGradeLevel( $content ) | ||
| ); |
There was a problem hiding this comment.
Just removed the $text_statistics variable here, it was not really necessary.
aristath
commented
Dec 13, 2023
Comment on lines
-434
to
+479
| if ( ! add_post_meta( $post_id, '_edac_summary', $summary, true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary', $summary ); | ||
| } | ||
|
|
||
| if ( ! add_post_meta( $post_id, '_edac_summary_passed_tests', $summary['passed_tests'], true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary_passed_tests', $summary['passed_tests'] ); | ||
| } | ||
|
|
||
| if ( ! add_post_meta( $post_id, '_edac_summary_errors', $summary['errors'], true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary_errors', $summary['errors'] ); | ||
| } | ||
|
|
||
| if ( ! add_post_meta( $post_id, '_edac_summary_warnings', $summary['warnings'], true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary_warnings', $summary['warnings'] ); | ||
| } | ||
|
|
||
| if ( ! add_post_meta( $post_id, '_edac_summary_ignored', $summary['ignored'], true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary_ignored', $summary['ignored'] ); | ||
| } | ||
|
|
||
| if ( ! add_post_meta( $post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'], true ) ) { | ||
| update_post_meta( $post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'] ); | ||
| } | ||
| update_post_meta( $post_id, '_edac_summary', $summary ); | ||
| update_post_meta( $post_id, '_edac_summary_passed_tests', $summary['passed_tests'] ); | ||
| update_post_meta( $post_id, '_edac_summary_errors', $summary['errors'] ); | ||
| update_post_meta( $post_id, '_edac_summary_warnings', $summary['warnings'] ); | ||
| update_post_meta( $post_id, '_edac_summary_ignored', $summary['ignored'] ); | ||
| update_post_meta( $post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'] ); |
There was a problem hiding this comment.
update_post_meta calls update_metadata.
Documentation for the update_metadata function mentions this:
If no value already exists for the specified object ID and metadata key, the metadata will be added.
With that in mind, calling add_post_meta before update_post_meta is unnecessary.
The resulting code is cleaner, faster, and easier to digest.
aristath
commented
Dec 13, 2023
Comment on lines
-470
to
+495
| if ( get_option( $option_name ) === false && EDAC_ANWW_ACTIVE ) { | ||
| if ( ! get_option( $option_name ) && EDAC_ANWW_ACTIVE ) { | ||
| update_option( $option_name, true ); | ||
| } elseif ( get_option( $option_name ) === true && ! EDAC_ANWW_ACTIVE ) { | ||
| } elseif ( get_option( $option_name ) && ! EDAC_ANWW_ACTIVE ) { |
There was a problem hiding this comment.
The edac_anww_update_post_meta option has a boolean value, so this condition was simplified
aristath
commented
Dec 13, 2023
Comment on lines
-512
to
+558
| return $rule['info_url'] . '?utm_source=accessibility-checker&utm_medium=software&utm_term=' . esc_attr( $rule['slug'] ) . '&utm_content=content-analysis&utm_campaign=wordpress-general&php_version=' . PHP_VERSION . '&platform=wordpress&platform_version=' . $wp_version . '&software=free&software_version=' . EDAC_VERSION . '&days_active=' . $days_active . ''; | ||
| return add_query_arg( | ||
| array( | ||
| 'utm_source' => 'accessibility-checker', | ||
| 'utm_medium' => 'software', | ||
| 'utm_term' => esc_attr( $rule['slug'] ), | ||
| 'utm_content' => 'content-analysis', | ||
| 'utm_campaign' => 'wordpress-general', | ||
| 'php_version' => PHP_VERSION, | ||
| 'platform' => 'wordpress', | ||
| 'platform_version' => $wp_version, | ||
| 'software' => 'free', | ||
| 'software_version' => EDAC_VERSION, | ||
| 'days_active' => $days_active, | ||
| ), | ||
| $rule['info_url'] | ||
| ); |
There was a problem hiding this comment.
Used add_query_arg here to build the URL instead of having a huge string with lots of concatenation.
add_query_arg is slightly slower, but in this case, the code becomes a lot easier to understand and maintain so it's worth the tradeoff.
aristath
commented
Dec 13, 2023
Comment on lines
-528
to
582
| if ( $simplified_summary && 'before' === $simplified_summary_position ) { | ||
| return $simplified_summary . $content; | ||
| } | ||
| if ( $simplified_summary && 'after' === $simplified_summary_position ) { | ||
| return $content . $simplified_summary; | ||
|
|
||
| if ( $simplified_summary ) { | ||
| if ( 'before' === $simplified_summary_position ) { | ||
| return $simplified_summary . $content; | ||
| } | ||
| if ( 'after' === $simplified_summary_position ) { | ||
| return $content . $simplified_summary; | ||
| } | ||
| } |
There was a problem hiding this comment.
Since both conditions depend on $simplified_summary, they were grouped/nested to make it clearer.
aristath
commented
Dec 13, 2023
Comment on lines
-582
to
+653
| $statement .= get_bloginfo( 'name' ) . ' ' . esc_html__( 'uses', 'accessibility-checker' ) . ' <a href="https://equalizedigital.com/accessibility-checker" target="_blank" aria-label="' . esc_attr__( 'Accessibility Checker', 'accessibility-checker' ) . ', opens a new window">' . esc_html__( 'Accessibility Checker', 'accessibility-checker' ) . '</a> ' . esc_html__( 'to monitor our website\'s accessibility. ', 'accessibility-checker' ); | ||
| $statement .= sprintf( | ||
| // translators: %1$s is the site name, %2$s is a link with the plugin name. | ||
| esc_html__( '%1$s uses %2$s to monitor our website\'s accessibility.', 'accessibility-checker' ), | ||
| get_bloginfo( 'name' ), | ||
| sprintf( | ||
| '<a href="https://equalizedigital.com/accessibility-checker" target="_blank" aria-label="%1$s">%2$s</a>', | ||
| esc_attr__( 'Accessibility Checker (opens in a new window)', 'accessibility-checker' ), | ||
| esc_html__( 'Accessibility Checker', 'accessibility-checker' ) | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| if ( $include_statement_link && $policy_page ) { | ||
| $statement .= esc_html__( 'Read our ', 'accessibility-checker' ) . '<a href="' . $policy_page . '">' . esc_html__( 'Accessibility Policy', 'accessibility-checker' ) . '</a>.'; | ||
| $statement .= sprintf( | ||
| // translators: %1$s is a link to the accessibility policy page, with text "Accessibility Policy". | ||
| esc_html__( 'Read our %s', 'accessibility-checker' ), | ||
| '<a href="' . $policy_page . '">' . esc_html__( 'Accessibility Policy', 'accessibility-checker' ) . '</a>.' | ||
| ); |
There was a problem hiding this comment.
Fixed concatenation in translation strings
SteveJonesDev
approved these changes
Dec 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains minor tweaks in the main plugin file:
add_post_metabeforeupdate_post_meta