Mfcttrf

I've created a simulator for the Premier League, which takes in all of the 20 teams and plays them against each other, recording the results and outputting a table at the end.

The skill levels are based off a prediction in an article by The Telegraph.

I am wondering whether or not my code can be improved, condensed or even added to with other features. Very new to coding so expecting lots of repetition/errors.

import math, random

# HIGHER RATED TEAM
higher = 1.148698355
# LOWER RATED TEAM
lower = 0.8705505633


# DEFINING THE TEAM CLASS
class Team:
 def __init__(self, name, skill):
 self.name = name
 self.skill = skill
 self.points = self.gf = self.ga = self.wins = self.draws = self.losses = 0

 def add_goals(self, goals):
 self.gf += goals


# INITIALISING ALL OF THE TEAMS - NAMES AND SKILL LEVELS
arsenal = Team("Arsenal", 16)
aston_villa = Team("Aston Villa", 6)
bournemouth = Team("AFC Bournemouth", 8)
brighton = Team("Brighton and Hove Albion", 5)
burnley = Team("Burnley", 4)
chelsea = Team("Chelsea", 17)
crystal_palace = Team("Crystal Palace", 11)
everton = Team("Everton", 14)
leicester = Team("Leicester City", 12)
liverpool = Team("Liverpool", 19)
man_city = Team("Manchester City", 20)
man_united = Team("Manchester United", 15)
newcastle = Team("Newcastle United", 3)
norwich = Team("Norwich City", 2)
sheffield_united = Team("Sheffield United", 1)
southampton = Team("Southampton", 7)
tottenham = Team("Tottenham Hotspur", 18)
watford = Team("Watford", 9)
west_ham = Team("West Ham United", 10)
wolves = Team("Wolverhampton Wanderers", 13)

# HOME/AWAY TEAMS FOR PLAYING EACH TIME AGAINST EACH OTHER
teams = [arsenal, aston_villa, bournemouth, brighton, burnley, chelsea,
 crystal_palace, everton, leicester, liverpool, man_city, man_united,
 newcastle, norwich, sheffield_united, southampton, tottenham, watford,
 west_ham, wolves]

# PRINT ALL TEAMS' NAME AND SKILL
for team in teams:
 print(team.name, team.skill)

# RANDOM SYSTEM FOR HOME GOALS
def home_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 raise ValueError

 if homeSkill > awaySkill:
 homeGoals = 0
 lambHome = higher ** (homeSkill - awaySkill)
 z = random.random()
 while z > 0:
 z = z - (((lambHome ** homeGoals) * math.exp(-1 * lambHome)) /
 math.factorial(homeGoals))
 homeGoals += 1
 return (homeGoals - 1)

 if homeSkill < awaySkill:
 homeGoals = 0
 lambHome = higher ** (homeSkill - awaySkill)
 z = random.random()
 while z > 0:
 z = z - (((lambHome ** homeGoals) * math.exp(-1 * lambHome)) /
 math.factorial(homeGoals))
 homeGoals += 1

 return (homeGoals - 1)

# RANDOM SYSTEM FOR AWAY GOALS
def away_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 return "Teams cannot play themselves!!!"

 if awaySkill > homeSkill:
 awayGoals = 0
 lambAway = lower ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lambAway ** awayGoals) * math.exp(-1 * lambAway)) /
 math.factorial(awayGoals))
 awayGoals += 1
 return (awayGoals - 1)

 if awaySkill < homeSkill:
 awayGoals = 0
 lambAway = lower ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lambAway ** awayGoals) * math.exp(-1 * lambAway)) /
 math.factorial(awayGoals))
 awayGoals += 1
 return (awayGoals - 1)

# LEAGUE SIZE AND SETTING UP THE LEAGUE
league_size = 20
POINTS = []
GOALS_FOR = []
GOALS_AGAINST = []
WINS =[]
DRAWS = []
LOSSES = []
for x in range(league_size):
 POINTS += [0]
 GOALS_FOR += [0]
 GOALS_AGAINST += [0]
 WINS += [0]
 DRAWS += [0]
 LOSSES += [0]

# PLAYING ALL TEAMS AGAINST EACH OTHER AND UPDATING STATISTICS
for x in range(league_size):
 print("========================================")
 print(teams[x].name + "'s home games: ")
 print("========================================")
 for y in range(league_size):
 error = 0
 try:
 homeScore = home_score(teams[x], teams[y])
 except ValueError:
 pass
 error += 1
 try:
 awayScore = away_score(teams[x], teams[y])
 except ValueError:
 pass
 if error == 0:
 print(teams[x].name, homeScore, ":", awayScore, teams[y].name)
 GOALS_FOR[x] += homeScore
 GOALS_FOR[y] += awayScore
 GOALS_AGAINST[x] += awayScore
 GOALS_AGAINST[y] += homeScore
 if homeScore > awayScore:
 WINS[x] += 1
 LOSSES[y] += 1
 POINTS[x] += 3
 elif homeScore == awayScore:
 DRAWS[x] += 1
 DRAWS[y] += 1
 POINTS[x] += 1
 POINTS[y] += 1
 else:
 WINS[y] += 1
 LOSSES[x] += 1
 POINTS[y] += 3
 else:
 pass

# ASSIGNING STATISTICS TO EACH TEAM
for x in range(league_size):
 teams[x].points = POINTS[x]
 teams[x].gf = GOALS_FOR[x]
 teams[x].ga = GOALS_AGAINST[x]
 teams[x].wins = WINS[x]
 teams[x].draws = DRAWS[x]
 teams[x].losses = LOSSES[x]

sorted_teams = sorted(teams, key=lambda t: t.points, reverse=True)


# PRITNING THE FINAL LEAGUE TABLE
print("| TEAM | POINTS | WINS | DRAWS | LOSSES | GOALS FOR | GOALS AGAINST |")
for team in sorted_teams:
 print("|",team.name," "*(24 - len(team.name)),"| ",team.points," "*(3 - len(str(team.points))),"| ",team.wins," "*(2 - len(str(team.wins))),"| ",
 team.draws," "*(2 - len(str(team.draws))),"| ",team.losses," "*(3 - len(str(team.losses))),"| ",team.gf," "*(4 - len(str(team.gf))),"| ",
 team.ga," "*(7 - len(str(team.ga))),"|")

Very nice program for a beginner! Great attempt! Here are some points.

Imports

For multiple imports for the same module, you use ,

from x_module import y, z

But for our purpose,

import math
import random

works better. Python is concerned about readability and space saving might not always be the best option.

Naming

Python follows a style convention known as PEP8. The normal variable naming convention can be summarised as:

• use snake_case instead of PascalCase or camelCase. my_pencil instead of MyPencil and myPencil
  


• use CAPITAL for constants. PI = 3.14, as the value of this variable won't change but like pencils = [], pencils will be reduced and expanded.

Some improvements

homeSkill -> home_skill

POINTS -> points as we see points being modified at POINTS += [0]

higher = 1.148698355 -> HIGHER = 1.148698355 since it is a constant

A note on objects

Since i see nowhere you needed to use the teams individually and since this is a simulation, teams could be defined as:

teams = [
 Team("Arsenal", 16),
 Team("Aston Villa", 6),
 Team("AFC Bournemouth", 8),
 ...,
 Team("Wolverhampton Wanderers", 13)
]

String formatting

The table can be simplified.
This

print("| TEAM | POINTS | WINS | DRAWS | LOSSES | GOALS FOR | GOALS AGAINST |")

can be written as

print("| :<20 | :^10 | :^10 | :^10 | :^10 | :^10 | :^10 |".format('TEAM',
 'POINTS', 'WINS', 'DRAWS', 'LOSSES', 'GOALS FOR', 'GOALS')

so that the following loop can be simplified to

for team in sorted_teams:
 print("| :<20 | :<10 | :<10 | :<10 | :<10 | :<10 | :<10 |".format(team.name, 
 team.points, team.wins, team.draws, team.losses, team.gf, team.ga))

where < means left align and ^ means align to the center.

Looping

league_size = 20
POINTS = []
GOALS_FOR = []
...
for x in range(league_size):
 POINTS += [0]
 ...

can be written as

league_size = 20
POINTS = [0] * league_size
GOALS_FOR = [0] * league_size
...

This looks good overall. For the code review, I'll start with general comments and then try to get into smaller details.

Documentation

Documentating the code looks like an easy task but doing it properly so that it actually adds interesting information without adding too much noise can be pretty hard.

Let's see what could be improved here.

For a start, you do not need to write the comments in upper case. it actually makes things harder to read.

In order to document a module, a class, a function, a method, you can use docstrings. You'll find more details about this in PEP 257 -- Docstring Conventions.

As you document the code, a pretty common tip is to say that Code Tells You How, Comments Tell You Why.

Here are a few instances:

• 
  # DEFINING THE TEAM CLASS tells you nothing that the code does not show. Having a docstring explaining the point of the Team class whould be more helpful.


• 
  # INITIALISING ALL OF THE TEAMS - NAMES AND SKILL LEVELS - here again, we can easily see that this is initialising teams.

Style

Python has a Style Guide called PEP 8. It could be a good idea to read it and try to see what could be applied to your code. A good example to start with could be the variable names.

You'll find various tools online to check your code compliancy to PEP 8 and/or to fix it.

Code organisation

It is good practice to split the definitions from your code such as functions and classes from the part of your code actually doing something when the script is called with an if __name__ == "__main__": guard.

This helps for the re-usability of your code (and also makes the organisation clearer/more conventional).

Improving home_score & away_score

These 2 functions are complicated, pretty long and fairly similar.
For all these reasons, there is probably something we can improve in them.

For a start, we could rename z into x to make the 2 functions even more similar and be able to see what differs from one to another.

Same for lambAway and lambHome renamed into lamb.

(Disclaimer: nothing here has been even remotely tested)

At this stage, we have:

def home_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 raise ValueError

 if homeSkill > awaySkill:
 goals = 0
 lamb = higher ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

 if homeSkill < awaySkill:
 goals = 0
 lamb = higher ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1

 return (goals - 1)

def away_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 raise ValueError

 if awaySkill > homeSkill:
 goals = 0
 lamb = lower ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

 if awaySkill < homeSkill:
 goals = 0
 lamb = lower ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

which is not really that much better.

Trying to factorise out the common parts from the if homeSkill > awaySkill and if homeSkill < awaySkill, it looks like we could have:

def home_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 raise ValueError

 goals = 0
 lamb = higher ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)


def away_score(home, away):
 homeSkill = home.skill / 3
 awaySkill = away.skill / 3

 if homeSkill == awaySkill:
 raise ValueError

 goals = 0
 lamb = lower ** (homeSkill - awaySkill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

But we could go further and extract the common parts of the function in a different function:

def home_score(home, away):
 return generate_random_score(home.skill / 3, away.skill / 3, higher)

def away_score(home, away):
 return generate_random_score(home.skill / 3, away.skill / 3, lower)

def generate_random_score(home_skill, away_skill, param):
 if home_skill == away_skill:
 raise ValueError

 goals = 0
 lamb = param ** (home_skill - away_skill)
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

It looks pretty good so far but we can still improve details.

For a start, it is clear now that lower and higher are not such great names. Maybe something mentionning home and away would be better.

Also, maybe the 2 methods do not really correspond to what you want: what you usually want is to simulate a full game and not just the score for a team. You could define a function returning a tuple:

def generate_random_score(home, away):
 delta_skill = (home - away) / 3
 return (generate_random_goal_number(delta_skill, higher), generate_random_goal_number(delta_skill, lower))


def generate_random_goal_number(delta_skill, param):
 if delta_skill == 0:
 raise ValueError

 goals = 0
 lamb = param ** delta_skill
 x = random.random()
 while x > 0:
 x = x - (((lamb ** goals) * math.exp(-1 * lamb)) / math.factorial(goals))
 goals += 1
 return (goals - 1)

A tiny improvement could be to rewrite:

x = x - long_expression

as:

x -= long_expression

Settings for the league

Havnig "league_size = 20" is a bit obscure and easy to break. You probably should have league_size = len(teams).

Also, in order to initialise the different arrays, you could have something like:

 GOALS_AGAINST = [0] * league_size

Finally, the team list could be initialised directly without defining so many variables that won't get reused.

You could write:

teams = [
 Team("Arsenal", 16),
 Team("Aston Villa", 6),
 Team("AFC Bournemouth", 8),
...
]

More code organisation

At the moment, you keep tracks of the stats for the teams in the class instances and in separate lists.

It would probably make sense to define a function/method simulate_game taking 2 teams as parameters and that would take care of generating a score and updating the team stats accordingly.

By the way, you do not necessarly need to keep track of the points. You could define a method in the Team objects to compute it on demand from the other statistics.

Special tip

I've said many things and there are still many things to say.

For learning purposes (and because I may have gotten things wrong in a few places), it could be a good idea to try to perform the changes described on your side.

Also, when random elements are involved, it can be hard to detect when you break something. My suggestion would be to initialise the random number generator with your favorite seed (for instance random.seed(42)), run your script, save the output and then keep that seed during your developments. If everything goes fine, the output should stay the same.

It does NOT exactly mean that:

• it the output stays the same, nothing got broken




• if the output changes, something got broken

but it does help to give you some confidence as you go.