HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Football Elo Rankings

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rankingsfootballelo

Problem

I have created the following code:

class Club(object):
    def __init__(self, name, points=1000):
        self.name = name
        self.points = points

    def __eq__(self, other):
        return self.name == other.name

    def __gt__(self, other):
        return self.points > other.points

    def compute_points(self, opponent, is_home, result):
        rating_diff = self.points - opponent.points + (100 if is_home else -100)
        expected = 1 / (1 + 10**(-rating_diff/400))
        return 32 * (result-expected)

def play_match(home, away, result):
    home_change = int(home.compute_points(away, True, result))
    away_change = int(away.compute_points(home, False, 1-result))
    home.points += home_change
    away.points += away_change

def club_index(club_to_find, clubs):
    index = 0
    while index < len(clubs):
        club = clubs[index]
        if club.name == club_to_find:
            return index
        index += 1
    return -1

def main():
    clubs = []
    with open("matches.txt") as file:
        matches = file.readlines()

    for match in matches:
        (home_str, away_str, result) = match.split(" ")

        index = club_index(home_str, clubs)
        if index == -1:
            home = Club(home_str)
            clubs.append(home)
        else:
            home = clubs[index]

        index = club_index(away_str, clubs)
        if index == -1:
            away = Club(away_str)
            clubs.append(away)
        else:
            away = clubs[index]

        play_match(home, away, float(result))

    clubs = sorted(clubs)
    clubs.reverse()
    with open("ranking.txt", "w") as file:
        for club in clubs:
            file.write(club.name + ": " + str(club.points) + "\n")

if __name__ == "__main__":
    main()


which takes a file containing match results, and creates a file containing a ranking based on the Elo Ranking System. However, I feel like my main() method could've been implemented in a better way, so I came here to s

Solution

In play_match, the Club class should be responsible for updating its own score. You could define an update_points method taking the same parameter as the computes_point one (and calling it). Also, shouldn't compute_points be responsible for the conversion to int ?

I find it a bit awkward to have the __eq__ and the __gt__ base their logic on completely different fields.

In a club_index : as a general rule, you do not iterate that way over object in Python.
The pythonic way to write

def club_index(club_to_find, clubs):
    index = 0
    while index < len(clubs):
        club = clubs[index]
        if club.name == club_to_find:
            return i
        index += 1
    return -1


is

def club_index(club_to_find, clubs):
    for i,club in enumerate(clubs)
        if club.name == club_to_find:
            return index
    return -1


Now, if I was to write such a program, I wouldn't even create a class ( you can have a look at http://www.youtube.com/watch?v=o9pEzgHorH0 ). A dictionnary to map club names to score is pretty much everything you need but that's a different way to see the problem I guess.

Edit : Actual tested code added.

def get_new_scores(home_score,away_score,result):
    rating_diff = home_score - away_score + 100
    home_expected = 1 / (1 + 10**(-rating_diff/400))
    away_expected = 1 / (1 + 10**(+rating_diff/400))
    home_return = int(32 * ((result  )-home_expected))
    away_return = int(32 * ((1-result)-away_expected))
    return home_score+home_return,away_score+away_return

def main():
    with open("club.txt") as file:
        matches = file.readlines()

    clubs = dict()
    for match in matches:
        (home_str, away_str, result) = match.split(" ")
        clubs[home_str],clubs[away_str] = get_new_scores(clubs.setdefault(home_str,1000), clubs.setdefault(away_str,1000), float(result))

    for club in sorted(clubs, key=clubs.get, reverse=True):
        print(club + ": " + str(clubs[club]) + "\n")

if __name__ == "__main__":
    main()

Code Snippets

def club_index(club_to_find, clubs):
    index = 0
    while index < len(clubs):
        club = clubs[index]
        if club.name == club_to_find:
            return i
        index += 1
    return -1
def club_index(club_to_find, clubs):
    for i,club in enumerate(clubs)
        if club.name == club_to_find:
            return index
    return -1
def get_new_scores(home_score,away_score,result):
    rating_diff = home_score - away_score + 100
    home_expected = 1 / (1 + 10**(-rating_diff/400))
    away_expected = 1 / (1 + 10**(+rating_diff/400))
    home_return = int(32 * ((result  )-home_expected))
    away_return = int(32 * ((1-result)-away_expected))
    return home_score+home_return,away_score+away_return

def main():
    with open("club.txt") as file:
        matches = file.readlines()

    clubs = dict()
    for match in matches:
        (home_str, away_str, result) = match.split(" ")
        clubs[home_str],clubs[away_str] = get_new_scores(clubs.setdefault(home_str,1000), clubs.setdefault(away_str,1000), float(result))

    for club in sorted(clubs, key=clubs.get, reverse=True):
        print(club + ": " + str(clubs[club]) + "\n")

if __name__ == "__main__":
    main()

Context

StackExchange Code Review Q#36927, answer score: 6

Revisions (0)

No revisions yet.