Microtask 2 #2
Microtask 2 #2
Conversation
App/views.py
Outdated
|
|
||
| percentile = 100-((userrank*100)/(totalusers+1)) | ||
| rating = (userrank/totalusers)*100 | ||
| if(rating<=1): |
There was a problem hiding this comment.
This big if...else structure could be written much more simply.
App/views.py
Outdated
| usr = request.POST['username'] | ||
| SqlQuery1 = "SELECT user_editcount FROM user WHERE user_name='" + usr + "';" | ||
| SqlQuery2 = "SELECT ss_total_edits FROM site_stats" | ||
| SqlQuery3 = "select rank from (select user_name, @curRank := @curRank + 1 as rank from user p, ( select @curRank := 0 ) q order by user_editcount DESC) q where user_name='"+usr+"';" |
There was a problem hiding this comment.
This does work but it's a bit overcomplicated. You just need to count how many users have more edits.
There was a problem hiding this comment.
More problematically, the first and third queries should escape the user. Right now you could create a user called something like '; DROP TABLE user; -- and things would end badly.
There was a problem hiding this comment.
I have improvised the rank query.
Also, I have made the input string 'sql injection proof' :)
App/views.py
Outdated
| totalusers_tup = curr.fetchall() | ||
|
|
||
| conn.close() | ||
| useredits = useredits_tup[0][0] |
There was a problem hiding this comment.
There is no guarantee this query returns any rows.
|
@tgr I have made all the changes as required. |
|
Tool link is - this |
App/views.py
Outdated
| flag = 1 | ||
| usr = request.POST['username'] | ||
| a = 1 if re.match("^[a-zA-Z0-9_ ]*$", s) else 0 | ||
| if(!a): |
There was a problem hiding this comment.
This doesn't look like valid Python. (Now that I look at it, all your ifs are in C-style. In Python you can just wrote if reqest.method=='POST' without the braces.)
Also, what's the point of the variable? You can always just inline it.
App/views.py
Outdated
| if(request.method=='POST'): | ||
| flag = 1 | ||
| usr = request.POST['username'] | ||
| a = 1 if re.match("^[a-zA-Z0-9_ ]*$", s) else 0 |
There was a problem hiding this comment.
This would leave users with a non-ASCII username stranded. You should accept anything that's a valid MediaWiki username and escape it. Usually in SQL that means using parametrized queries. (Also, where does the s come from?)
App/views.py
Outdated
|
|
||
| def main2(request): | ||
| if(request.method=='POST'): | ||
| flag = 1 |
There was a problem hiding this comment.
This works, but if you want a nicer approach, you can wrap everything in a try...catch and throw an exception when the user is not found etc.
App/views.py
Outdated
| usr = request.POST['username'] | ||
| a = 1 if re.match("^[a-zA-Z0-9_ ]*$", s) else 0 | ||
| if(!a): | ||
| flag=0 |
There was a problem hiding this comment.
Not much point in setting this as you are exiting the function anyway.
|
@tgr Please review. |
App/views.py
Outdated
| totaledits = totaledits_tup[0][0] | ||
| userrank = userrank_tup[0][0] | ||
| totalusers = totalusers_tup[0][0] | ||
| percentile = 100-((userrank*100)/(totalusers+1)) |
There was a problem hiding this comment.
I don't understand the logic here. In practice this will be the same as 100 - rating (since totalusers is large so adding one doesn't matter). In theory, the +1 seems wrong. The percentile is the smallest number N for which the user is included in the top N% of users - you order the users by editcount, split the list into 100 equal chunks, if the user is in the chunk with the largest editcount, they are in the first percentile, the next chunk is the second etc. So the top 1% is the users with rank 1..(totalusers/100), top 2% is (totalusers/100)..(2*totalusers/100) etc. So percentile is same as what you call rating here (or 100 minus that if you prefer - either could be called a percentile).
There was a problem hiding this comment.
I was calculating percentile on the basis no of editcounts , so the user with highest editcounts gets 99 percentile (since 99% of users lie below him) and not the basis of userrank. In which case, the percentile of the user with highest no of editcounts would we 1, so he would be in top 1 percentile.
I'll have updated it according to userrank.
|
@tgr sorry sir, for the fault and delay. Please review. |
@tgr Please review my PR for microtask2.