4
\$\begingroup\$

I wrote code to plot a rating distribution, which looks like this:

enter image description here

The most important function is the stats function:

function stats($songID){
    $st=$this->conn->prepare("SELECT
        (SELECT COUNT(*) FROM list WHERE SongID=:songID),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='10'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='9.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='9'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='8.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='8'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='7.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='7'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='6.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='6'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='5.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='4.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='4'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='3.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='3'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='2.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='2'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='1.5'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='1'),
        (SELECT COUNT(*) FROM list WHERE SongID=:songID AND Rating='0.5')
        FROM list WHERE SongID=:songID");
    $st->bindparam(":songID",$songID);
    $st->execute();
    return $st->fetchall();
}


function bars($rate,$percentage,$ratings){
    return <<<HTML
    <div class="progress border border-dark w-100 bg-transparent border-0 mt-1">
            <span style="width:20px;" class="font-weight-bold text-light">
                {$rate}
            </span>
            <div class="progress-bar border border-dark bg-light" role="progressbar" style="max-width:25%;width:{$percentage}%;" aria-valuenow="{$ratings}" aria-valuemin="0" aria-valuemax="100"></div>
            <span class="font-weight-bold text-light ml-1 mr-1">
                {$percentage}%({$ratings} votes)
            </span>
        </div>
HTML;
}

And then in another file I use these methods:

$stats=$song->stats($_GET['songid']);
    foreach($stats as $key){
        $total=$key[0];
        $rate=10;
        for($i=1;$i<21;$i++){
            echo $song->bars($rate,round(($key[$i]/$total)*100,2),$key[$i]);
            $rate-=0.5;
            }
            break;
        }

This code works perfectly, but somehow I'm not satisfied with the stats function, the way I solved it. It's just too long, and doesn't look quite good to me.

So the question is: is there a way to make that function a bit shorter, so I wouldn't repeat the same thing 20 times but with different values for the Rating column?

I was thinking that this might actually be the only way to do it, but I thought I'd give it a try here, since there are way smarter programmers here than I am.

\$\endgroup\$
  • 8
    \$\begingroup\$ Just to make the question clear, please post the schema definition for the list table as well. \$\endgroup\$ – 200_success Oct 9 at 18:38
6
\$\begingroup\$

It would be better to use GROUP by in your SQL. By this way you will get rid of long SQL query.

SELECT Rating, COUNT(id) FROM list WHERE songID=:songID GROUP BY Rating;
\$\endgroup\$
2
\$\begingroup\$

Assuming that you are trying to extract ALL of the rating values from your table, I agree with Unnamed that you should simply GROUP BY and return the Rating, and the COUNT() in the result set.

Two things to add here...

  1. Without apply default values, you need to be aware that Unnamed's query will potentially deliver gaps where there is no count for a particular Rating score. To mitigate this, you should array_merge() your own set of defaults (all of the expected Rating scores) and assign them with a zero count or modify your looping process to provide 0 counts for missing Ratings.

  2. GROUP BY has a cool feature that will provide the grand total for you as well (which seems perfect for your use case and is not included in unnamed's solution)-- ROLL UP: http://www.mysqltutorial.org/mysql-rollup/

    This means you can do something like:

    function stats($songID){
        $st = $this->conn->prepare(
            "SELECT COALESCE(Rating, 'Total') Rating, COUNT(1) `Count`
             FROM list
             WHERE songID = ?
             GROUP BY Rating DESC WITH ROLLUP"
        );
    
        return $st->execute([$songID])->fetchall();
    }
    

    Here's a fiddle demo: https://www.db-fiddle.com/f/9WZJwBunnEHjy6MMQGYSbt/4

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.