Optimize PHP code for Wordpress shortcode (Star-Rating)

Question

I am using WP with the Avada theme and the built in Star Rating element as well as ACF Pro. Unfortunately I found out that the Star-Rating element adds a rating scheme to the footer of the website without asking. There is also no way to disable this. This is annoying

So I created my own shortcode based on the CSS code of the Star-Rating element.

It also works perfectly. Nevertheless, I have the feeling that there might still be potential for optimization.

The CSS code works as follows:

Example rating: 2.0 stars out of max 5.0

2x stars (filled)
3x stars (empty)

Example rating: 2.5 stars from max 5.0

2x stars (filled)
1x star (percentage filled: 50%)
2 stars (empty)

Example rating: 1.2 stars from max 5.0

1x star (filled)
1x star (percentage filled: 20%)
3x stars (empty)

Example rating: 4.6 stars from max 5.0

4x stars (filled)
1x star (percentage filled: 60%)

I have the following code now:

function show_star_rating_review()
{
    ob_start();
    global $post;

    $rating   = get_field('rating', $post);
    $rate_arr = explode('.',$rating);

    if ($rate_arr[1]) {
    $rate_emp = 4 - $rate_arr[0];
    } else {
    $rate_emp = 5 - $rate_arr[0];
    }

  $x = 1;
  $y = 1;

    echo '<div class="awb-stars-rating awb-stars-rating-1 awb-stars-rating-no-text" aria-label="Rating: ' .  $rating . ' out of 5">';
  echo '<style>.awb-stars-rating-1 .awb-stars-rating-filled-icon{margin-right:2px;color:#ffd041;}.awb-stars-rating-1 .awb-stars-rating-empty-icon,.awb-stars-rating-1 .awb-stars-rating-partial-icon-wrapper{margin-right:2px;}.awb-stars-rating-1 .awb-stars-rating-partial-icon{color:#ffd041;}.awb-stars-rating-1 .awb-stars-rating-icons-wrapper{color:#dbdbdb;font-size:23px;}</style>';
    echo '<div class="awb-stars-rating-icons-wrapper">';
  while ($x <= $rate_arr[0]) {
    $x++;
    echo '<i class="fa-star fas awb-stars-rating-filled-icon"></i>';
  }
    if ($rate_arr[1]) {
    echo '<i class="fa-star fas awb-stars-rating-partial-icon-wrapper"><i class="fa-star fas awb-stars-rating-partial-icon" style="width:' . $rate_arr[1] . '0%;"></i></i>';
    }
  if ($rate_emp > 0) {
    while ($y <= $rate_emp) {
      $y++;
      echo '<i class="fa-star fas awb-stars-rating-empty-icon"></i>';
        }
    }
    echo '</div>';
    echo '</div>';

    return ob_get_clean();
}
add_shortcode('ShowStarRatingReview', 'show_star_rating_review');

$rating is the ACF with the assigned rating. $rate_arr decomposes the rating into number before and number after the point.

If number after the point exists, so x.x rating then empty stars = 4 - number before the point. If number after the point exists, so x.0 rating then empty stars = 5 - number before the point.

As I mentioned before, the code works without problems so far. I'm just wondering if maybe I've made the whole thing too complicated.

Answer 1

I don't know if I am getting older and grumpier or older and wiser, but I find that I am increasingly in favor of writing code that is easier to read and easier to manage.

First, I found your css styling block very difficult to read. In general, I'd say move all of those style declarations to an external style sheet, but with Wordpress (I don't use Wordpress), you may prefer to jam all of that into your shortcode function.

Second, I generally prefer to avoid string concatenation -- especially when creating html markup. Since learning about printf(), I find myself using it (and its related native functions) to create "template strings" with placeholders, then adding the the variables at the end of the function call. This creates a clear separation from markup and variables which improves maintainability.

I kicked around the idea of making str_repeat() calls versus a for() loop with math. I thought about building a string of star markup versus an array of star markups. I am not completely sold on what is "best" and I could be convinced of several different "good" approaches. This is a rather lightweight task in terms of processing and memory, so there isn't any benefit in trying to optimize for performance. Striving for code brevity is only going to damage readability. I'll share one version that I mocked up which endeavors to make the code easier to read/maintain.

Code: (Demo)

function show_star_rating_review(): string
{
    global $post;
    $rating = get_field('rating', $post);
    [$whole, $decimal] = explode('.', $rating);    

    $css = <<<CSS
        <style>
        .awb-stars-rating-1 .awb-stars-rating-filled-icon{
            margin-right:2px;
            color:#ffd041;
        }
        .awb-stars-rating-1 .awb-stars-rating-empty-icon,
        .awb-stars-rating-1 .awb-stars-rating-partial-icon-wrapper{
            margin-right:2px;
        }
        .awb-stars-rating-1 .awb-stars-rating-partial-icon{
            color:#ffd041;
        }
        .awb-stars-rating-1 .awb-stars-rating-icons-wrapper{
            color:#dbdbdb;
            font-size:23px;
        }
        </style>
    CSS;

    $stars['whole'] = str_repeat("<i class=\"fa-star fas awb-stars-rating-filled-icon\"></i>\n                 ", $whole);
    if ($decimal) {
        $stars['piece'] = sprintf(
            "<i class=\"fa-star fas awb-stars-rating-partial-icon-wrapper\">
                     <i class=\"fa-star fas awb-stars-rating-partial-icon\" style=\"width:%d0%%;\"></i>
                 </i>\n                 ",
             $decimal
        );
    }
    $stars['empty'] = str_repeat("<i class=\"fa-star fas awb-stars-rating-empty-icon\"></i>\n                 ", 5 - ceil($rating));

    return sprintf(
        '%s
         <div class="awb-stars-rating awb-stars-rating-1 awb-stars-rating-no-text" aria-label="Rating: %s out of 5">
             <div class="awb-stars-rating-icons-wrapper">
                 %s
             </div>
         </div>',
        $css,
        $rating,
        implode($stars)
    );
}

Answer 2

Speaking strictly about the code and not the approach in general.

Use array destructuring to get nicer, more readable variables:

[$whole, $decimal] = explode('.', $rating);

Some syntactic sugar to replace if/else:

$rate_emp = ($decimal ? 4 : 5) - $whole;

Loops can be replaced by str_repeat(). Optionally, you can check if repeat count is less than 1 and avoid the function call if it is. If there's a chance that rating could be above 5.0, this is necessary to avoid errors.

echo str_repeat('<i class="fa-star fas awb-stars-rating-filled-icon"></i>', $whole);

Generating and catching the output seems unnecessary, especially since you're doing it using PHP syntax. Write to string directly:

function show_star_rating_review()
{
    global $post;

    $rating = get_field('rating', $post);
    [$whole, $decimal] = explode('.', $rating);

    $rate_emp = ($decimal ? 4 : 5) - $whole;

    $output = '<div class="awb-stars-rating awb-stars-rating-1 awb-stars-rating-no-text" aria-label="Rating: ' . $rating . ' out of 5">'
    . '<style>.awb-stars-rating-1 .awb-stars-rating-filled-icon{margin-right:2px;color:#ffd041;}.awb-stars-rating-1 .awb-stars-rating-empty-icon,.awb-stars-rating-1 .awb-stars-rating-partial-icon-wrapper{margin-right:2px;}.awb-stars-rating-1 .awb-stars-rating-partial-icon{color:#ffd041;}.awb-stars-rating-1 .awb-stars-rating-icons-wrapper{color:#dbdbdb;font-size:23px;}</style>'
    . '<div class="awb-stars-rating-icons-wrapper">'
    . str_repeat('<i class="fa-star fas awb-stars-rating-filled-icon"></i>', $whole);

    if ($decimal) {
      $output .= '<i class="fa-star fas awb-stars-rating-partial-icon-wrapper"><i class="fa-star fas awb-stars-rating-partial-icon" style="width:' . $decimal . '0%;"></i></i>';
    }

    $output .= str_repeat('<i class="fa-star fas awb-stars-rating-empty-icon"></i>', $rate_emp)
    . '</div>'
    . '</div>';

    return $output;
}