13
JS96
3y

I was lazy tonight and wanted to implement something of this kind very fast… is this really dumb or okay in your opinion?

If it's dumb, do you have a better and cleaner solution?

Comments
  • 6
    Does case check for fuzzy equality? It will probably never accurately match a float value. But I don't know if PHP does any kind of checks for this. Maybe give star value an integer number. Like 0 to 10. There may be a more programmatic approach to the stars too. Like maybe index the star strings in an array and index them.
  • 11
    Looks like it sucks. The rating should be an integer counted in half-stars, and it should be determined via a formula instead of a switch-case.
  • 1
    Loop from 1 to the number of stars, append a whole star for each iteration, append a half star if the score is x.5, then subtract the score (round to nearest whole number) from 5 and use that value to loop from 1 to that value to generate empty stars
  • 9
    @Demolishun @JS96

    Why so complicated.

    You've a rating.

    It's an integer and a possible rest.

    The integer can be looped. For each loop call function getFullStar.

    Determine rest.

    For each step (if it was eg a quarter instead of half) of the rest call the other function.
  • 4
    You don't even have to come up with a better way to know that there definitely is one (or more likely, many).

    Repeated code like this is a clear code smell, but hey - we've all been in a rush! :)
  • 3
    @Demolishun php will yes.

    1.5 and 1.50000 are the same unless you use
    switch(true) case x === 1.5

    However, couldn't you just grab the int and modulus the remainder and grab the start and 1 half if there's a remainder?
  • 2
    I get it that I can of course just loop a few things, but don't find a real reason to, especially for something that is always max 5 steps as in this case and isn't dynamic at all.

    @Fast-Nop what do you mean by "via a formula"?

    I'm looking for a clean solution but that doesn't require useless allocation of memory (looping, etc.) at the same time.

    What's wrong with using float instead of integers?
  • 4
    @JS96 The float thing is because float comparisons for equality will break often. This is due to not being able to represent values exactly in the float format. So you check a relative range of values based upon the appropriate magnitude of your values. But it sounds like php may be able to.
  • 2
  • 2
    @JS96 wtf. This code sucks to read and it's performance is going to be the same as

    starCount = (int)rating
    halfStar =(rating - starCount) !== 0

    For starcount getstar;
    If halfstar gethalfstar;

    For rating - starcount - (int)halfstar getemptystar;

    Come one dude. That's not lazy, you gotta be trolling :D
  • 0
    @arekxv I like it!
  • 3
    @JS96 Floats can have rounding errors. You never test for equality on floats in any language (that's a strict anti-pattern), only for intervals, including half-open ones.

    And yeah, that can involve a loop. I can see why you object to that on performance grounds, but then a string lookup table instead of a switch-case is even better. Works easily if you convert the float rating to a half star based integer right before the lookup.
  • 1
  • 2
    Builder for the function series that produce the output, and a map of predicate -> markup factory would be a lot cleaner.
  • 1
    function getStars($arg){
    // Init variable constants
    $half_star = "fas fa-star-half-alt";
    $full_star = "fas fa-star";
    $empty_star = "far fa-star";
    $star_count = 5;
    $🌟 = "";

    /* Loop through the sky and
    * and pick up the lonely stars
    **/
    for($i = 0; $i < $star_count; $i++){
    $star_class = $empty_star;
    if($arg - $i >= 1) $star_class = $full_star;
    else if($arg - i >= 0.5) $star_class = $half_star;

    // Pickup star with style
    $🌟 += "<div class='col'>
    <i class='"+ $star_class + "'></i>
    "</div>";
    }

    return $🌟; // and go home.
    }
  • 0
    Make your ratings out of ten, make two "half star" graphics (or even a single mirrored half star) and use a simple iterator with modulus to determine which half to render.
  • 0
    Personally thing you can extract the rating so that the integer before the . (Decimal point) determines full stars and after determines half stars and since the amount is five stars you fill the rest with empty stars, a function to do that maybe
  • 1
    @pandasama or @nitwhiz solution looks very clean and concise too
Add Comment