views:

69

answers:

5

Hi folks

OK - I'll get straight to the point - here's the PHP code in question:

<h2>Highest Rated:</h2>

    <?php 

        // Our query base               
        $query = $this->db->query("SELECT * FROM code ORDER BY rating DESC");

        foreach($query->result() as $row) {
    ?>  
        <h3><?php echo $row->title." ID: ";echo $row->id; ?></h3>
            <p class="author"><?php  $query2 = $this->db->query("SELECT email FROM users WHERE id = ".$row->author);
echo $query2->row('email');?></p>
            <?php   echo ($this->bbcode->Parse($row->code)); ?>

        <?php } ?>

Sorry it's a bit messy, it's still a draft. Anyway, I researched ways to use a Ratings system - previously I had a single 'rating' field as you can see by SELECT * FROM code ORDER BY rating DESC. However I quickly realised calculating averages like that wasn't feasible, so I created five new columns - rating1, rating2, rating3, rating4, rating5. So when 5 users rating something 4 stars, rating4 says 5... does that make sense? Each ratingx column counts the number of times the rating was given.

So anyway: I have this SQL statement:

SELECT id, (ifnull(rating1,0) + ifnull(rating2,0) + ifnull(rating3,0) + ifnull(rating4,0) + ifnull(rating5,0)) / ((rating1 IS NOT NULL) + (rating2 IS NOT NULL) + (rating3 IS NOT NULL) + (rating4 IS NOT NULL) + (rating5 IS NOT NULL)) AS average FROM code

Again messy, but hey. Now what I need to know is how can I incorporate that SQL statement into my script? Ideally you'd think the overall query would be 'SELECT * FROM code ORDER BY (that really long query i just stated) DESC' but I can't quite see that working... how do I do it? Query, store the result in a variable, something like that?

If that makes no sense sorry! But I really appreciate the help :)

Jack

+4  A: 

You should go back to the drawing board completely.

<?php
$query = $this->db->query("SELECT * FROM code ORDER BY rating DESC");
foreach($query->result() as $row) {
    $this->db->query("SELECT email FROM users WHERE id = ".$row->author;
}

Anytime you see this in your code, stop what you're doing immediately. This is what JOINs are for. You almost never want to loop over the results of a query and issue multiple queries from within that loop.

SELECT code.*, users.email 
FROM code
JOIN users ON users.id = code.author
ORDER BY rating DESC

This query will grab all that data in a single resultset, removing the N+1 query problem.

I'm not addressing the rest of your question until you clean up your question some and clarify what you're trying to do.

hobodave
I'd also just echo out the simple things like `H3` from PHP and not bounce between html and PHP all the time for those trivial things
KM
why was this downvoted? this looks like sensible advice to me?!
Roland Bouman
Thank you for your advice! I am still learning, but thank you, I really appreciate it :) I guess we all have to begin somewhere :P
Jack Webb-Heller
And Roland, all seemed to be downvoted. I voted everything back up to 0, I agree with you...
Jack Webb-Heller
+1  A: 

First, I'd decide whether your application is write-heavy or read-heavy. If there are a lot more reads than writes, then you want to minimize the amount of work you do on reads (like this script, for example). On the assumption that it's read-heavy, since most webapps are, I'd suggest maintaining the combined average in a separate column and recalculating it whenever a user adds a new rating.

Other options are:

  • Try ordering by the calculated column name 'average'. SQL Server supports this. . not sure about mysql.
  • Use a view. You can create a view on your base table that does the average calculation for you and you can query against that.

Also, unrelated to your question, don't do a separate query for each user in your loop. Join the users table to the code table in the original query.

sidereal
Thanks - it's going to be read-heavy, definitely. Thank you for your help - as you can tell I'm still learning :)
Jack Webb-Heller
+2  A: 

if you would like to change your tables again, here is my suggestion:

why don't you store two columns: RatingTotal and RatingCount, each user that rates it will increment RatingCount by one, and whatever they vote (5,4,4.2, etc) is added to RatingTotal. You could then just ORDER BY RatingTotal/RatingCount

also, I hope you store which users rated each item, so they don't vote multiple times! and swing the average their way.

KM
Thanks, knowing me I'd have forgotten to incorporate checking which users have already voted ;)
Jack Webb-Heller
create a rating table, with user, item, rating, and a date in it. you could use this to calculate average rating with, however storing the aggregate in the item can simplify and speed up your other queries.
KM
+1  A: 

You should include it in the SELECT part:

SELECT *, (if ....) AS average FROM ... ORDER BY average

Edit: assuming that your ifnull statement actually works...

You might also want to look into joins to avoid querying the database again for every user; you can do everything in 1 select statement.

Apart from that I would also say that you only need one average and the number of total votes, that should give you all the information you need.

jeroen
+1  A: 

Some excellent ideas, but I think the best way (as sidereal said that it's more read heavy that write heavy) would be to have columns rating and times_rated, and just do something like this:

new_rating = ((times_rated * rating) + current_rating) / (times_rated + 1)

current_rating being the rating being applied when the person clicks the little stars. This simply weights the current user's rating in an average with the current rating.

Jonah Bron
Thanks, great idea!
Jack Webb-Heller