School House Points (Python + SQLite)

Question

I am creating a database to store house points for our school running events. I would like to be able to tally all the points for the houses and it is working fine, however, I think it may be a bit verbose at this point.

Is there a simpler/better way with maybe more efficient code?

Races are stored in a races table (competitorID, raceID, time).

Student data is stored in a competitor table (competitorID, names, house).

The code will query the DB and bring back unique races, and then assign a point value based on their position (calculated by their race time).

import sqlite3
conn = sqlite3.connect('house.db')
conn.row_factory = sqlite3.Row
# House Totals
blueTotal = 0
greenTotal = 0
redTotal = 0
yellowTotal = 0
# Assign points from 1st = 15 through to 10-Last being 1
def points():
 global pointvalue
 if count == 0:
 pointvalue = 15
 places()
 if count == 1:
 pointvalue = 12
 places()
 if count == 2:
 pointvalue = 10
 places()
 if count == 3:
 pointvalue = 8
 places()
 if count == 4:
 pointvalue = 7
 places()
 if count == 5:
 pointvalue = 6
 places()
 if count == 6:
 pointvalue = 5
 places()
 if count == 7:
 pointvalue = 4
 places()
 if count == 8:
 pointvalue = 3
 places()
 if count == 9:
 pointvalue = 2
 places()
 if count > 10:
 pointvalue = 1
 places()
#Add points to houses
def places():
 global blueTotal, greenTotal, redTotal, yellowTotal, pointvalue
 if competitors['house'] == "blueTotal":
 blueTotal += pointvalue
 if competitors['house'] == "redTotal":
 redTotal += pointvalue
 if competitors['house'] == "yellowTotal":
 yellowTotal += pointvalue
 if competitors['house'] == "greenTotal":
 greenTotal += pointvalue
for x in range(1,50):
 competitorDetails = conn.execute('SELECT firstname, surname, house '
 'FROM race, competitor '
 'WHERE competitor.competitorID = race.competitorID '
 'AND race.raceID = ' + str(x))
 count=0
 for competitors in competitorDetails:
 points()
 count += 1
print("blueTotal " + str(blueTotal))
print("greenTotal " + str(greenTotal))
print("redTotal " + str(redTotal))
print("yellowTotal " + str(yellowTotal)) 

• aghast: I don't see any mention of the time being used, as you described in your preliminary text. You should update your SQL to include this. Also, much of this work can be done in SQL rather than Python. Can you explain why you're using Python for this instead?

Answer 1

DRY

This repetitive code is an anti-pattern:

blueTotal = 0
greenTotal = 0
redTotal = 0
yellowTotal = 0

Please define

houses = 'blue green red yellow'.split()

and then you can use array access to perform "the same action" across all houses:

for house in houses:
 total[house] = 0

One could also assign total = collections.defaultdict(int), but that would be a horse of a different color. (https://docs.python.org/3/library/collections.html#collections.defaultdict)

global

The global keyword is usually not your friend, it leads to unfortunate coupling. Here, it is offering you a hint that you want to define a class, set totals to zero in the __init__() constructor, and then have places() access self.total[].

arg passing

This is a bit crazy:

 for competitors in competitorDetails:
 points()

Yes, you can treat competitors as an implicit argument by making it global, but there is absolutely no reason to, and the current code makes it extremely difficult for readers to understand what is going on. Please, please make competitors an explicit argument, passing it in with points(competitors).

formatting

Clearly this works:

print("blueTotal " + str(blueTotal))

but the explicit call to str is slightly verbose. Consider re-phrasing such print statements in one of these ways:

print("blueTotal", blueTotal)
print(f'blueTotal {blueTotal}')

• aghast: Don't you mean "a house of a different color"? ;-)
• J_H: It's an idiom that, among other places, shows up in Oz. It means "quite a different thing", and implicit init to zero for all potential houses seemed quite different from OP's repetitive code. idioms.thefreedictionary.com/a+horse+of+a+different+color , oz.fandom.com/wiki/Horse_of_a_Different_Color
• DoctorDep: @J_H ... Woosh.
• Alex F: Why did you choose to say houses = 'blue green red yellow'.split() instead of houses = ['blue', 'green', 'red', 'yellow']? The second one seems more explicit to me...
• J_H: It's a concise and common idiom. For N items, I can choose to put N blanks between them, or 4*N punctuation characters, which can mean the difference between a one-line or multi-line expression. As N gets bigger, so does the relative advantage.

Answer 2

Here are some hints on how to decrease the size of your code and make it more Pythonic.

Stack Ifs

Anytime you see stacked ifs, that are all basically the same, you should consider using a dict. In the case here, the primary thing the ifs were doing was mapping a race place to points. So let's do that explicitly like:

pointvalues = { 
 0: 15,
 1: 12,
 2: 10,
 3: 8,
 4: 7,
 5: 6,
 6: 5,
 7: 4,
 8: 3,
 9: 2,
 10: 1,
}

Use a dict for tracking like things

Instead of the specially named globals variables for tracking points, you can use a dict, and use your house names as keys to the dict like:

# House Totals
totals = dict(
 blueTotal=0,
 greenTotal=0,
 redTotal=0,
 yellowTotal=0,
)

Or you might use setdefault to save having to init the houses names at all.

Use enumerate:

Instead of explicitly counting loop iterations, you should use the builtin enumerate like:

for place, competitors in enumerate(competitorDetails):
 # Add points to houses
 totals[competitors['house']] += pointvalues.get(place, 0)

Also the above shows how to use the two dict's we built earlier.

• Jörg W Mittag: A dict with consecutive integer keys starting at 0 is isomorphic to a list.

Answer 3

A few things no one has touched on yet...

points()

Your points values follow a pattern which can be easily modeled mathematically:

# Assign points from 1st = 15 through to 10-Last being 1
def points():
 global pointvalue
 if count == 0:
 point_value = 15
 places()
 elif count < 3:
 point_value = 14 - 2 * count
 places()
 elif count < 11:
 point_value = 11 - count
 places()
 elif count > 10:
 point_value = 1
 places()

On my machine, this improvement made this section of the program about 2x faster.

if-elif

Since competitors['house'] can only have one value, there is no sense in checking if it is all values with repetitive if statements. Instead, use an if-elif structure or perhaps if-elif-elif-else.

# add points to houses
def places():
 global blue_total, green_total, red_total, yellow_total, point_value
 if competitors['house'] == "blue_total":
 blue_total += point_value
 elif competitors['house'] == "red_total":
 red_total += point_value
 elif competitors['house'] == "yellow_total":
 yellow_total += point_value
 elif competitors['house'] == "green_total":
 green_total += point_value

Style:

Python's own style guide dictates a convention for function and variable names: names should be lowercase, with words separated by underscores as necessary to improve readability.

Therefore: blueTotal -> blue_total and pointvalue -> point_value.