6
\$\begingroup\$

I'm new to Python and have written a UEFA Champions League draw simulator. It receives input for 32 teams from 14 countries, and then randomly distributes them into groups of 4, never putting two teams from the same country into the same group.

I am looking for more efficient ways to write this program, or other suggestions on how to improve my code.

If you have any questions as to why I did certain things, just ask.

# Champions League Draw

# global variables
num_teams = 32
england = []
france = []
spain = []
germany = []
italy = []
portugal = []
russia = []
dutch = []
turkey = []
swiss = []
scotland = []
greece = []
poland = []
belgium = []

groupA = []
groupB = []
groupC = []
groupD = []
groupE = []
groupF = []
groupG = []
groupH = []
groups = [groupA, groupB, groupC, groupD, groupE, groupF, groupG, groupH]
countries1 = [england, france, spain, germany, italy, portugal, russia]
countries2 = [dutch, turkey, swiss, scotland, greece, poland, belgium]
used = []
loading = 0
tens = 10

# intro
print("\tUEFA Champions League Draw Simulator\n")
print("Enter teams to make your very own UEFA Champions League.")
input("Press the enter key to begin.")

# ask for teams
england += [input("\nPlease enter an English team: ")]
england += [input("Please enter an English team: ")]
england += [input("Please enter an English team: ")]
england += [input("Please enter an English team: ")]

france += [input("Please enter a French team: ")]
france += [input("Please enter a French team: ")]
france += [input("Please enter a French team: ")]
france += [input("Please enter a French team: ")]

spain += [input("Please enter a Spanish team: ")]
spain += [input("Please enter a Spanish team: ")]
spain += [input("Please enter a Spanish team: ")]
spain += [input("Please enter a Spanish team: ")]

germany += [input("Please enter a German team: ")]
germany += [input("Please enter a German team: ")]
germany += [input("Please enter a German team: ")]
germany += [input("Please enter a German team: ")]

italy += [input("Please enter an Italian team: ")]
italy += [input("Please enter an Italian team: ")]
italy += [input("Please enter an Italian team: ")]
italy += [input("Please enter an Italian team: ")]

portugal += [input("Please enter a Portugese team: ")]
portugal += [input("Please enter a Portugese team: ")]
portugal += [input("Please enter a Portugese team: ")]

russia += [input("Please enter a Russian team: ")]
russia += [input("Please enter a Russian team: ")]

dutch += [input("Please enter a Dutch team: ")]

turkey += [input("Please enter a Turkish team: ")]

swiss += [input("Please enter a Swiss team: ")]

scotland += [input("Please enter a Scottish team: ")]

greece += [input("Please enter a Greek team: ")]

poland += [input("Please enter a Polish team: ")]

belgium += [input("Please enter a Belgiun team: ")]

# make copies
england1 = england[:]
france1 = france[:]
spain1 = spain[:]
germany1 = germany[:]
italy1 = italy[:]
portugal1 = portugal[:]
russia1 = russia[:]
dutch1 = dutch[:]
turkey1 = turkey[:]
swiss1 = swiss[:]
scotland1 = scotland[:]
greece1 = greece[:]
poland1 = poland[:]
belgium1 = belgium[:]

countries3 = [england1, france1, spain1, germany1, italy1, portugal1, russia1]
countries4 = [dutch1, turkey1, swiss1, scotland1, greece1, poland1, belgium1]

# create groups
import random
while num_teams != 7:
    x = 0
    position = random.randrange(len(countries1))
    country = countries1[position]
    country1 = countries3[position]
    if country1:
        team = random.choice(country1)
        if team not in used:
            group = random.choice(groups)
            if len(group) < 4:
                for i in group:
                    if i not in country:
                        x += 0
                    else:
                        x += 1
                if x == 0:
                    group += [team]
                    num_teams -= 1
                    used += [team]
                    country1.remove(team)
                    loading += 1
                    if loading == tens:
                        print("\nLoading...\n")
                        tens += 10

while num_teams != 0:
    x = 0
    position = random.randrange(len(countries2))
    country = countries2[position]
    country1 = countries4[position]
    if country1:
        team = random.choice(country1)
        if team not in used:
            group = random.choice(groups)
            if len(group) < 4:
                for i in group:
                    if i not in country:
                        x += 0
                    else:
                        x += 1
                if x == 0:
                    group += [team]
                    num_teams -= 1
                    used += [team]
                    country1.remove(team)
                    loading += 1
                    if loading == tens:
                        print("\nLoading...\n")
                        tens += 10

# display groups
print("\nAssembling groups...\n")
input("Groups complete. Press the enter key to view.")

print("\nGroup A")
print(groupA[0])
print(groupA[1])
print(groupA[2])
print(groupA[3])

print("\nGroup B")
print(groupB[0])
print(groupB[1])
print(groupB[2])
print(groupB[3])

print("\nGroup C")
print(groupC[0])
print(groupC[1])
print(groupC[2])
print(groupC[3])

print("\nGroup D")
print(groupD[0])
print(groupD[1])
print(groupD[2])
print(groupD[3])

print("\nGroup E")
print(groupE[0])
print(groupE[1])
print(groupE[2])
print(groupE[3])

print("\nGroup F")
print(groupF[0])
print(groupF[1])
print(groupF[2])
print(groupF[3])

print("\nGroup G")
print(groupG[0])
print(groupG[1])
print(groupG[2])
print(groupG[3])

print("\nGroup H")
print(groupH[0])
print(groupH[1])
print(groupH[2])
print(groupH[3])

# exit
input("\n\nPress the enter key to exit.")
\$\endgroup\$
  • \$\begingroup\$ I agree with the posters above. However just as an idea... this is A LOT of manual input. Perhaps starting with a GUI or some options for the user to select from is the better way to go \$\endgroup\$ – MattR Aug 11 '17 at 19:01
  • \$\begingroup\$ I am rewriting your code and currently I have it down to less than 100 lines of code. I am running into a problem though. What exactly are you trying to accomplish with you while loops? If I know that I could finish my example. \$\endgroup\$ – Mike - SMT Aug 11 '17 at 21:12
  • \$\begingroup\$ I wanted to make a while loop that would place teams in each group until all the teams had been used. So whenever a team is placed in a group, one is subtracted from num_teams. Eventually num_teams will be equal to 0, all the teams will be in a group, and the loop will end. Hopefully this helps. \$\endgroup\$ – MuckinAround145 Aug 11 '17 at 22:05
  • \$\begingroup\$ Ok that helps a little. What is the reason for having each team placed in each group? Is it just random or is there a reason for the groups? \$\endgroup\$ – Mike - SMT Aug 11 '17 at 22:31
  • \$\begingroup\$ I'm trying to randomly put the teams in groups of four, but without having any two teams be from the same country. It's a simulation of the Champions League draw in which someone physically draws balls with the team names from a bowl, similar to a lottery. There is a rule that no group can contain two teams from the same country. \$\endgroup\$ – MuckinAround145 Aug 11 '17 at 22:56
3
\$\begingroup\$

A good practice of writing code would to be to follow the DRY method. AKA "Don't Repeat Yourself".

To accomplish this instead of writing out each line for each input request you can use a for loop and if statements to accomplish just about anything you need here.

When you are going to have many variables being generated you should use a list or a dictionary to store each variable as it helps keep your code shorter and makes it much easier to manage the date.

For example Instead of the 27 lines of code you are using to create your 2 countries list countries1 and countries2 you can accomplish this with less than half the lines of code using a for loop like this one below:

countries_list = ["england", "france", "spain", "germany", "italy", "portugal", "russia",
"dutch", "turkey", "swiss", "scotland", "greece", "poland", "belgium"]

groups = []
countries1 = []
countries2 = []

for i in range(7):
    groups.append([])
    countries1.append([])
    countries2.append([])

Now we have a countries1 and countries2 list with the same functionality as your original code and we can manipulate date by providing the index to where we want to check or change data.

Now instead of the 45 lines of code you use to ask for team names you can get the same results with 8 lines of code using a for loop like the below code:

# ask for teams
for i in range(7):
    for x in range(4):
        countries1[i].append([input("\nPlease enter an {} team: ".format(countries_list[i]))])

for i in range(7,14):
    countries2[i-7].append([input("\nPlease enter an {} team: ".format(countries_list[i]))])

You can apply this same logic to all parts of your code that are repeating so you take up less code and can manage the code a bit better.

If you want to make things super easy you could create a few functions that do many of the same task and that take a few arguments so you can get even more done with less code written.

Once you have all this down you may want to look into the OOP (Object Oriented Programing) style as it has many advantages over what you are currently doing.

One last example:

If you use this logic throughout your code you will end up making other parts of your code easier to write. For example if you do what I have shown you so far your last part of the code (47 lines of print statements) can be reduced to 5ish lines of code like this:

group_list = ["\nGroup A", "\nGroup B", "\nGroup C", "\nGroup D",
              "\nGroup E", "\nGroup F", "\nGroup G", "\nGroup H"]
for i in group:
    print(group_list[i])
    for x in group[i]:
        print(x)

With all the options I have listed we can create a program that works the way you want with less than 50 lines of code.

UPDATE: Here is the example I promised from the comments.

The below example will be 60 lines of code but you can reduce the lines by reformatting the dictionaries in the first 14 lines. to just be on 2 lines instead of 8 lines and removing white space in the rest of the code.

import random
import math


num_teams = 32
country_list = {"england" : ["English", 4], "france" : ["French", 4],
                "spain" : ["Spanish", 4], "germany" : ["German", 4],
                "italy" : ["Italian", 4], "portugal" : ["Portuguese", 3],
                "russia" : ["Russian", 2], "dutch" : ["Dutch", 1],
                "turkey" : ["Turkish", 1], "scotland" : ["Scottish", 1],
                "greece" : ["Greek", 1], "poland" : ["Polish", 1],
                "belgium" : ["Belgian", 1]}
group_and_teams = {"GroupA" : {}, "groupB" : {}, "groupC" : {}, "groupD" : {},
                   "groupE" : {}, "groupF" : {}, "groupG" : {}, "groupH" : {}}

print("\tUEFA Champions League Draw Simulator\n")
print("Enter teams to make your very own UEFA Champions League.")
input("Press the enter key to begin.")
x = 0
for country in country_list:
    x += country_list[country][1]
x = math.ceil(x / len(group_and_teams))

list_teams = {}
for country in country_list:
    for x in range(country_list[country][1]):
        list_teams[input("Please enter an {} team: ".format(country_list[country][0]))] = country

def find_open_slot(new_team):
    good = True
    group_chosen = False
    rand = random.sample(list(group_and_teams), 1)[0]
    while group_chosen == False:
        if len(group_and_teams[rand]) > 0:
            for listed_team in group_and_teams[rand]:
                if new_team == listed_team or list_teams[new_team][1] == group_and_teams[rand][listed_team]:
                    good = False
        else:
            group_chosen = True
            return rand

        if good == False:
            rand = random.sample(list(group_and_teams), 1)[0]
        else:
            group_chosen = True
            return rand

for team in list_teams:
    group = find_open_slot(team)
    teams = group_and_teams[group]
    teams[team] = list_teams[team]

print("\nAssembling groups...\n")
input("Groups complete. Press the enter key to view.")

for group in group_and_teams:
    for teams in group_and_teams[group]:
        print("{}: team: {}, Country: {}".format(group,teams, group_and_teams[group][teams]))

input("\n\nPress the enter key to exit.")
\$\endgroup\$
  • \$\begingroup\$ Just a question, why do you need the second for statement, for x in range(4), after the first one, for i in range(7). \$\endgroup\$ – MuckinAround145 Aug 11 '17 at 22:20
  • \$\begingroup\$ @MuckinAround145 Well your code has the first 7 countries ask for a team 4 times each and the last 7 countries only ask for 1 team name so I made 2 separate loops for that. Thought I could have write in an if statement to the first loop that could have taken care of that for me but I didn't want to get 2 complicated with my example. \$\endgroup\$ – Mike - SMT Aug 11 '17 at 22:30
  • \$\begingroup\$ @MuckinAround145: I have updated my answer with an example of how you could write your code to be shorter and more DRY and also removing around 75% of the lines of code from your original post. \$\endgroup\$ – Mike - SMT Aug 14 '17 at 17:33
1
\$\begingroup\$

The first, very small improvenment:

Instead of

england += [input("\nPlease enter an English team: ")]
england += [input("Please enter an English team: ")]
england += [input("Please enter an English team: ")]
england += [input("Please enter an English team: ")]

(and similar) use a loop:

print
for __ in range(4):
    england += [input("Please enter an English team: ")]

Note the two successive underline characters (__) instead of an unused variable name.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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