Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The code should be relatively self-explanatory. I was wondering how much rep the Top Voters on sites did "generate" on a site, in comparison to how much rep they "own".

Therefore I created a little SEDE-Query and I am now wondering, if I follow good SQL-Practices and how the performance of the query could possibly be improved (even though it's rather simple already..)

WITH Voters AS(
   SELECT DISTINCT UpVotes, DownVotes, Id, Reputation
   FROM Users
   WHERE Reputation != 101 AND Reputation != 1 --Exclude users without repchanges
   )

SELECT TOP ##topX:int?20##
    Users.Id as [UserLink],
    Users.DisplayName as [SortName],
    Voters.Upvotes * 8 as [EstimatedAddedRep], 
    --as there is no split between question and answer votes
    -- we take 8 as median in accordance to the tendency that there are more
    -- answer votes (10 rep) than question votes (5 rep)
    Voters.Downvotes * 3 as [EstimatedDestroyedRep],
    Users.Reputation as [OwnedRep],
    Voters.Upvotes * 8 / Users.Reputation as [Ratio]
FROM Users, Voters
WHERE users.id = Voters.Id
GROUP BY Users.Id, Users.DisplayName, Users.Reputation, Voters.UpVotes, Voters.DownVotes
ORDER BY EstimatedAddedRep DESC

For anyone who wants to run the newest query, you can do so here: Rep "Created" vs Rep "owned" by topX voters

share|improve this question
    
I think the ratio column name could be better, is lower better or higher better. Since I have the lowest score, does that make me a vampire on CR, or more like a blood donor ? :) –  konijn May 16 at 15:33
    
It would be nice to account for bounties awarded as well. –  200_success May 16 at 17:12

4 Answers 4

up vote 3 down vote accepted

First of all, when you are selecting and want to perform mathematics on it you should directly specify what you want done

instead of

Voters.Upvotes * 8 as [EstimatedAddedRep], 

it should be

(Voters.Upvotes * 8) AS [EstimatedAddedRep]     

And

Voters.Upvotes * 8 / Users.Reputation as [Ratio]

Should be

((Voters.Upvotes * 8) / (Users.Reputation)) as [Ratio]

you should always specify with parenthesis when performing arithmetic in code, because it doesn't always do what you think it will do.


I don't do this with my FROM and WHERE Statements

FROM Users, Voters
WHERE users.id = Voters.Id

I would do this

FROM Users INNER JOIN Voters ON Users.Id = Voters.Id

and leave out the WHERE clause.


For the issue brought up in Chat you would just need to add it in like this (simple query so not an issue to repeat yourself in the select)

((Voters.Upvotes * 8) / Users.Reputation) AS RepCreated --?????

Check all your Columns and Tables as well, you have some weird casing issues where sometimes you capitalize and sometimes you don't, it was hard to figure out what was right

share|improve this answer
    
Is there a performance difference in using WHERE versus INNER JOIN? –  Edward May 16 at 15:19
    
I don't know @Edward, I don't write my queries like that. it makes more sense to call it a join, because that is really what you are doing (joining 2 tables). This answer says that it is the same thing, stackoverflow.com/a/354834/1214743 –  Malachi May 16 at 15:36
1  
@Edward - "Is there a performance difference in using WHERE versus INNER JOIN?" No. The optimiser will recognise them as equivalent. The form using WHERE is sorta-deprecated. The ON style offers more clarity. –  Michael Green May 18 at 11:17

I may be mistaken but I was under the impression that !=was not good SQL practice, rather <> is preferrable.

e.g.

WHERE Reputation <> 101 AND Reputation <> 1 --Exclude users without repchanges
share|improve this answer
    
I think that is just a matter of preference, I think that != doesn't work in some SQL engines but for Stack Exchange (SEDE) which is a SQL Server either will work I believe. –  Malachi May 16 at 17:33
1  
I just checked the manual, looks like they both work but != is not ISO standard. technet.microsoft.com/en-us/library/ms188074.aspx –  Phrancis May 16 at 17:46

Although it makes little (almost no) difference to the SQL engine, it might be more friendly to the user to produce the resulting ratio as a floating point number instead of an integer (probably rounded to only one or two places after the decimal point).

This is particularly true for any user whose rep is (even slightly) higher than the amount it estimates they've created. Showing all such users as having a ratio of 0 basically leaves it to them to do the math in their head to figure out some idea of the real ratio.

share|improve this answer

Incorporing the changes suggested by @Malachi and @JerryCoffin, as well as a little tweaking by myself, the query currently looks as follows:

WITH Voters AS(
   SELECT DISTINCT 
       UpVotes,
       DownVotes,
       Id,
       Reputation,
       --as there is no split between question and answer votes
        -- we take 8 as median in accordance to the tendency that there are more
        -- answer votes (10 rep) than question votes (5 rep)
      (UpVotes * 8) AS EstimatedAddedRep,
       --(-2 Rep) for the downvoted, 1 rep "cost"
      (DownVotes * 3) AS EstimatedDestroyedRep
   FROM Users
   WHERE Reputation != 101 AND Reputation != 1 --Exclude users without repchanges
   )

SELECT TOP ##topX:int?20##
    Users.Id as [User Link],
    Users.DisplayName as [SortName],
    Voters.EstimatedAddedRep, 
    Voters.EstimatedDestroyedRep,
    Users.Reputation as [OwnedRep],
    round((Voters.EstimatedAddedRep * 1.0 / Users.Reputation), 2) as [CreatedToOwnedRatio]
FROM Users INNER JOIN Voters ON Users.Id = Voters.Id
GROUP BY Users.Id, Users.DisplayName,
    Users.Reputation, Voters.EstimatedAddedRep, Voters.EstimatedDestroyedRep
ORDER BY EstimatedAddedRep DESC

I have moved the calculations for EstimatedAddedRep and EstimatedDestroyedRep to the Voters to remove clutter in the actual SELECT-Statement.

Additionally I renamed the Ratio column to better reflect what it means.

share|improve this answer
    
I like how you put more into the CTE like the EstimatedAddedRep that probably made it a bit faster –  Malachi May 16 at 16:51

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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