Convert rating value to visible stars using fontawesome icons

Question

Just curious if there is a more elegant way to perform the same result. I have a calculated rating which I want to convert to visible stars using fontawesome icons. Max rating is 5. I am rounding the rating to the nearest .5. Done in js given a rating called data (sum of all votes / number of votes).

//example, can be anything from 0-5 after being computed
data = 3.65;

//get rating in nearest .5 value (this is how many stars need to be yellow)
rating = yellow = (Math.round(data*2)/2).toFixed(1);

//how many stars need to be white
white = 5 - rating;

output = '<div title="'+rating+'">';

//output full yellow stars
while (yellow >= 1) {
    output += '<i class="fa fa-star text-yellow"></i>';
    yellow--;
}
//what is left can either be .5 or 0... if .5 output half yellow star
if (yellow == .5) {
    output += '<i class="fa fa-star-half-o text-yellow"></i>';
}
//output remaining full white stars
while (white >= 1) {
    output += '<i class="fa fa-star-o text-yellow"></i>';
    white--;
}

return output + '</div>';

The above does what I want it to, but I have to believe there is a better approach to get the same result.

Answer 1

I recently implemented this same thing, here's what I used. Main difference: Let the for loop do the math for you. By the end of the first loop, if there is half a star it will be in the i variable. Use an array to buffer your string (or create DOM objects instead, don't use += to build a string).

document.getElementById("stars").innerHTML = getStars(3.6);

function getStars(rating) {

  // Round to nearest half
  rating = Math.round(rating * 2) / 2;
  let output = [];

  // Append all the filled whole stars
  for (var i = rating; i >= 1; i--)
    output.push('<i class="fa fa-star" aria-hidden="true" style="color: gold;"></i>&nbsp;');

  // If there is a half a star, append it
  if (i == .5) output.push('<i class="fa fa-star-half-o" aria-hidden="true" style="color: gold;"></i>&nbsp;');

  // Fill the empty stars
  for (let i = (5 - rating); i >= 1; i--)
    output.push('<i class="fa fa-star-o" aria-hidden="true" style="color: gold;"></i>&nbsp;');

  return output.join('');

}

<link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet" />

<b>Rating: <span id=stars></span></b>

Answer 2

The algorithm looks good to me. What I would change:

Use for loops when iterating over a previously known range or sequence

for ( ; yellow >= 1; yellow--)
  output += '<i class="fa fa-star text-yellow"></i>';
// ...
for ( ; white >= 1; white--)
  output += '<i class="fa fa-star-o text-yellow"></i>';

for-loops are better at communicating what is being iterated over and there's a common recommendation to not alter the iteration counter outside of the loop header. Both of these individually and even more so together make it much easier to read and understand loops.

Of course some algorithms are more elegant and easier to understand if you disregard these recommendations. That's why they're recommendations for the majority of simple cases and not hard rules.

Use String.prototype.repeat() to repeat strings

Instead of the loops above you can more elegantly (and probably more efficiently) use:

output += '<i class="fa fa-star text-yellow"></i>'.repeat(yellow);
// ...
output += '<i class="fa fa-star-o text-yellow"></i>'.repeat(white);

Manipulate DOM objects instead of generating HTML code in JavaScript

This may be a personal preference but I wouldn't even bother with HTML here.

var output = document.createElement("div");
output.title = rating;

for ( ; yellow >= 1; yellow--)
{
  var star = document.createElement("i");
  star.className = "fa fa-star text-yellow";
  output.appendChild(star);
}
if (yellow == .5) {
  var star = document.createElement("i");
  star.className = "fa fa-star-half-o text-yellow";
  output.appendChild(star);
}
for ( ; white >= 1; white--)
{
  var star = document.createElement("i");
  star.className = "fa fa-star-o text-yellow";
  output.appendChild(star);
}

return output;

This will directly construct the DOM (sub-)tree that you're trying to describe in HTML.

Respect the intent behind HTML tags

<i> is intended for text set in italic. However the use of “physical” formats in HTML is discouraged in favour of cascading styles (CSS). Use “logical” formats like <em> or <strong> or the other suggested tags on the previously linked MDN page to designate your intent and format them with CSS (or trust the client web browser to use a sensible default if you care about intent but not the actual resulting typesetting). If none of the pre-defined intents match your case, use <span> and annotate it with classes, then format it with CSS.