Simulate round-robin tournament draw Planned maintenance scheduled April 23, 2019 at 23:30 UTC (7:30 pm US/Eastern) Announcing the arrival of Valued Associate #679: Cesar Manara Unicorn Meta Zoo #1: Why another podcast?Fast draw wxPython OnPaintRuby program to simulate a game of Narcotic SolitaireSimulate object.__getattribute__ in PythonPython list dictionary items round robin mixingDraw a directory treeUEFA Champions League Draw SimulatorCoding exercise to simulate animal populationTournament Management modelAxelrod TournamentIteratively simulate a parasite population
Are there existing rules/lore for MTG planeswalkers?
Will I lose my paid in full property
What's the difference between using dependency injection with a container and using a service locator?
Processing ADC conversion result: DMA vs Processor Registers
Why would the Overseers waste their stock of slaves on the Game?
Does Prince Arnaud cause someone holding the Princess to lose?
The 'gros' functor from schemes into (strictly) locally ringed topoi
Why does Java have support for time zone offsets with seconds precision?
Does using the Inspiration rules for character defects encourage My Guy Syndrome?
Why do people think Winterfell crypts is the safest place for women, children & old people?
Is there an efficient way for synchronising audio events real-time with LEDs using an MCU?
When I export an AI 300x60 art board it saves with bigger dimensions
What is /etc/mtab in Linux?
Co-worker works way more than he should
How would it unbalance gameplay to rule that Weapon Master allows for picking a fighting style?
Why I cannot instantiate a class whose constructor is private in a friend class?
Like totally amazing interchangeable sister outfit accessory swapping or whatever
Determinant of a matrix with 2 equal rows
How would you suggest I follow up with coworkers about our deadline that's today?
Is there a possibility to generate a list dynamically in Latex?
Married in secret, can marital status in passport be changed at a later date?
Raising a bilingual kid. When should we introduce the majority language?
Is it accepted to use working hours to read general interest books?
What's parked in Mil Moscow helicopter plant?
Simulate round-robin tournament draw
Planned maintenance scheduled April 23, 2019 at 23:30 UTC (7:30 pm US/Eastern)
Announcing the arrival of Valued Associate #679: Cesar Manara
Unicorn Meta Zoo #1: Why another podcast?Fast draw wxPython OnPaintRuby program to simulate a game of Narcotic SolitaireSimulate object.__getattribute__ in PythonPython list dictionary items round robin mixingDraw a directory treeUEFA Champions League Draw SimulatorCoding exercise to simulate animal populationTournament Management modelAxelrod TournamentIteratively simulate a parasite population
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
I decided to implement the round robin algorithm in Python. My code takes a list of teams as input and prints the schedule.
This is my first try to write something by my own after taking some online courses, so I am absolutely sure that this code must be significantly improved.
Here it is:
import random
def simulate_draw(teams):
if len(teams) % 2 == 0:
simulate_even_draw(teams)
else:
simulate_odd_draw(teams)
def simulate_even_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
def simulate_odd_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
if len(teams)+1 not in r:
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
I think that big blocks of code that largely repeat themselves inside 2 functions may be united in one function, but not sure how to implement it.
python simulation
New contributor
$endgroup$
add a comment |
$begingroup$
I decided to implement the round robin algorithm in Python. My code takes a list of teams as input and prints the schedule.
This is my first try to write something by my own after taking some online courses, so I am absolutely sure that this code must be significantly improved.
Here it is:
import random
def simulate_draw(teams):
if len(teams) % 2 == 0:
simulate_even_draw(teams)
else:
simulate_odd_draw(teams)
def simulate_even_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
def simulate_odd_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
if len(teams)+1 not in r:
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
I think that big blocks of code that largely repeat themselves inside 2 functions may be united in one function, but not sure how to implement it.
python simulation
New contributor
$endgroup$
$begingroup$
I think your code is missing an entry point (if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.
$endgroup$
– C. Harley
8 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can callsimulate_draw
with a list of names of your liking if you want to test it.
$endgroup$
– Mathias Ettinger
6 hours ago
add a comment |
$begingroup$
I decided to implement the round robin algorithm in Python. My code takes a list of teams as input and prints the schedule.
This is my first try to write something by my own after taking some online courses, so I am absolutely sure that this code must be significantly improved.
Here it is:
import random
def simulate_draw(teams):
if len(teams) % 2 == 0:
simulate_even_draw(teams)
else:
simulate_odd_draw(teams)
def simulate_even_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
def simulate_odd_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
if len(teams)+1 not in r:
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
I think that big blocks of code that largely repeat themselves inside 2 functions may be united in one function, but not sure how to implement it.
python simulation
New contributor
$endgroup$
I decided to implement the round robin algorithm in Python. My code takes a list of teams as input and prints the schedule.
This is my first try to write something by my own after taking some online courses, so I am absolutely sure that this code must be significantly improved.
Here it is:
import random
def simulate_draw(teams):
if len(teams) % 2 == 0:
simulate_even_draw(teams)
else:
simulate_odd_draw(teams)
def simulate_even_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
def simulate_odd_draw(teams):
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = random.sample(gm, len(gm))
if len(teams)+1 not in r:
print(dic[r[0]-1] + ' plays ' + dic[r[1]-1])
I think that big blocks of code that largely repeat themselves inside 2 functions may be united in one function, but not sure how to implement it.
python simulation
python simulation
New contributor
New contributor
edited 8 hours ago
Eldar
New contributor
asked 8 hours ago
EldarEldar
313
313
New contributor
New contributor
$begingroup$
I think your code is missing an entry point (if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.
$endgroup$
– C. Harley
8 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can callsimulate_draw
with a list of names of your liking if you want to test it.
$endgroup$
– Mathias Ettinger
6 hours ago
add a comment |
$begingroup$
I think your code is missing an entry point (if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.
$endgroup$
– C. Harley
8 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can callsimulate_draw
with a list of names of your liking if you want to test it.
$endgroup$
– Mathias Ettinger
6 hours ago
$begingroup$
I think your code is missing an entry point (
if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.$endgroup$
– C. Harley
8 hours ago
$begingroup$
I think your code is missing an entry point (
if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.$endgroup$
– C. Harley
8 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can call
simulate_draw
with a list of names of your liking if you want to test it.$endgroup$
– Mathias Ettinger
6 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can call
simulate_draw
with a list of names of your liking if you want to test it.$endgroup$
– Mathias Ettinger
6 hours ago
add a comment |
1 Answer
1
active
oldest
votes
$begingroup$
Making the code testable and tested
The first step to improve your code is to try to make it testable. By doing so, you usually have to deal with Separation of Concerns: in your case, you have to split the logic doing the output from the logic computing games. The easiest way to do so it to rewrite slightly the simulate_XXX
functions to return values instead of writing them.
Once it it done, you can easily write tests for the function computing the games (in order to make this easier to implement, I've extracted out the randomising part as well).
At this stage, we have something like:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
Now we can start improving the code in a safer way.
Remove what's not required
dic[i+1] = ''
is not required, we can remove it.
Also, resetting zipped
to the empty string is not required, we can remove it. Maybe we could get rid of zipped
altogether.
Finally, we call for gm in list(game)
when game
is already a list. We can remove the call to list
.
Loop like a native
I highly recommend Ned Batchelder's talk "Loop like a native" about iterators. One of the most simple take away is that whenever you're doing range(len(iterable)), you can probably do things in a better way: more concise, clearer and more efficient.
In your case, we could have:
for i in range(len(teams)):
dic[i] = teams[i]
replaced by
for i, team in enumerate(teams):
dic[i] = team
And we could do:
for _ in teams:
instead of
for i in range(len(teams))
(Unfortunately, this can hardly be adapted to the "even" situation)
Note: "_" is a usual variable names for values one does not plan to use.
Dict comprehension
The dictionnary initiation you perform via dict[index] = value
in a loop could be done using the Dictionnary Comprehension syntactic sugar.
Instead of:
dic =
for i, team in enumerate(teams):
dic[i] = team
we you can write:
dic = i: team for i, team in enumerate(teams)
Now it is much more obvious, it also corresponds to:
dic = dict(enumerate(teams))
Finally, we can ask ourselves how we use this dictionnary: the answer is "to get the team at a given index". Do we really need a dictionnay for this ? I do not think so. We can get rid of the dic
variable and use teams
directly.
At this stage, we have:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
The right tool for the task
The part:
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
could/should probably be written with pop:
arr2.pop(0)
arr1.pop()
And now, these line can be merged with arrXX.append(arrYYY[ZZ])
:
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
games.append(list(zip(arr1, arr2)))
Removing useless steps
A loop is used to fill an array. Another one is used to iterate over the array. We could try to use a single loop to do everything (disclaimer: this is not always a good idea as far as readability goes).
This removes the need for a few calls to list
.
At this stage, we have:
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
if len(teams)+1 not in gm:
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
Better indices
You generate a list of indices using i+1
and then use val - 1
when you use them. You can make your life easier twice.
Iterable unpacking
Instead of using indices to get elements from an iterable with a know number of elements, you can use iterable unpacking.
You'd get
def simulate_even_draw(teams):
"""Return the list of games."""
half_len = int(len(teams)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams))][::-1]
matches = []
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
matches.append((teams[a], teams[b]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
half_len = int((len(teams)+1)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams)+1)][::-1]
matches = []
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
if len(teams) not in (a, b):
matches.append((teams[a], teams[b]))
return matches
True divisions
Instead of using "/" and convert the float result to int, you can use "//" which is an integer division.
Other way to compute indices
We could write something like:
indices = list(range(len(teams)))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
and
indices = list(range(len(teams)+1))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
Altough, if we don't care about order, we could use the more direct:
arr1 = indices[:half_len]
arr2 = indices[half_len:]
Remove the duplicated logic
Don't repeat yourself is a principle of software development that you could easily apply here. Indeed, we have 2 functions that look very similar.
This is trickier than expected and I have to go. I may continue another day.
Batteries included
The Python standard library contains many useful things. Among them, we have the very interesting module itertools
which itself contains combinations which is what you want.
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Eldar is a new contributor. Be nice, and check out our Code of Conduct.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217969%2fsimulate-round-robin-tournament-draw%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Making the code testable and tested
The first step to improve your code is to try to make it testable. By doing so, you usually have to deal with Separation of Concerns: in your case, you have to split the logic doing the output from the logic computing games. The easiest way to do so it to rewrite slightly the simulate_XXX
functions to return values instead of writing them.
Once it it done, you can easily write tests for the function computing the games (in order to make this easier to implement, I've extracted out the randomising part as well).
At this stage, we have something like:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
Now we can start improving the code in a safer way.
Remove what's not required
dic[i+1] = ''
is not required, we can remove it.
Also, resetting zipped
to the empty string is not required, we can remove it. Maybe we could get rid of zipped
altogether.
Finally, we call for gm in list(game)
when game
is already a list. We can remove the call to list
.
Loop like a native
I highly recommend Ned Batchelder's talk "Loop like a native" about iterators. One of the most simple take away is that whenever you're doing range(len(iterable)), you can probably do things in a better way: more concise, clearer and more efficient.
In your case, we could have:
for i in range(len(teams)):
dic[i] = teams[i]
replaced by
for i, team in enumerate(teams):
dic[i] = team
And we could do:
for _ in teams:
instead of
for i in range(len(teams))
(Unfortunately, this can hardly be adapted to the "even" situation)
Note: "_" is a usual variable names for values one does not plan to use.
Dict comprehension
The dictionnary initiation you perform via dict[index] = value
in a loop could be done using the Dictionnary Comprehension syntactic sugar.
Instead of:
dic =
for i, team in enumerate(teams):
dic[i] = team
we you can write:
dic = i: team for i, team in enumerate(teams)
Now it is much more obvious, it also corresponds to:
dic = dict(enumerate(teams))
Finally, we can ask ourselves how we use this dictionnary: the answer is "to get the team at a given index". Do we really need a dictionnay for this ? I do not think so. We can get rid of the dic
variable and use teams
directly.
At this stage, we have:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
The right tool for the task
The part:
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
could/should probably be written with pop:
arr2.pop(0)
arr1.pop()
And now, these line can be merged with arrXX.append(arrYYY[ZZ])
:
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
games.append(list(zip(arr1, arr2)))
Removing useless steps
A loop is used to fill an array. Another one is used to iterate over the array. We could try to use a single loop to do everything (disclaimer: this is not always a good idea as far as readability goes).
This removes the need for a few calls to list
.
At this stage, we have:
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
if len(teams)+1 not in gm:
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
Better indices
You generate a list of indices using i+1
and then use val - 1
when you use them. You can make your life easier twice.
Iterable unpacking
Instead of using indices to get elements from an iterable with a know number of elements, you can use iterable unpacking.
You'd get
def simulate_even_draw(teams):
"""Return the list of games."""
half_len = int(len(teams)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams))][::-1]
matches = []
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
matches.append((teams[a], teams[b]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
half_len = int((len(teams)+1)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams)+1)][::-1]
matches = []
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
if len(teams) not in (a, b):
matches.append((teams[a], teams[b]))
return matches
True divisions
Instead of using "/" and convert the float result to int, you can use "//" which is an integer division.
Other way to compute indices
We could write something like:
indices = list(range(len(teams)))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
and
indices = list(range(len(teams)+1))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
Altough, if we don't care about order, we could use the more direct:
arr1 = indices[:half_len]
arr2 = indices[half_len:]
Remove the duplicated logic
Don't repeat yourself is a principle of software development that you could easily apply here. Indeed, we have 2 functions that look very similar.
This is trickier than expected and I have to go. I may continue another day.
Batteries included
The Python standard library contains many useful things. Among them, we have the very interesting module itertools
which itself contains combinations which is what you want.
$endgroup$
add a comment |
$begingroup$
Making the code testable and tested
The first step to improve your code is to try to make it testable. By doing so, you usually have to deal with Separation of Concerns: in your case, you have to split the logic doing the output from the logic computing games. The easiest way to do so it to rewrite slightly the simulate_XXX
functions to return values instead of writing them.
Once it it done, you can easily write tests for the function computing the games (in order to make this easier to implement, I've extracted out the randomising part as well).
At this stage, we have something like:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
Now we can start improving the code in a safer way.
Remove what's not required
dic[i+1] = ''
is not required, we can remove it.
Also, resetting zipped
to the empty string is not required, we can remove it. Maybe we could get rid of zipped
altogether.
Finally, we call for gm in list(game)
when game
is already a list. We can remove the call to list
.
Loop like a native
I highly recommend Ned Batchelder's talk "Loop like a native" about iterators. One of the most simple take away is that whenever you're doing range(len(iterable)), you can probably do things in a better way: more concise, clearer and more efficient.
In your case, we could have:
for i in range(len(teams)):
dic[i] = teams[i]
replaced by
for i, team in enumerate(teams):
dic[i] = team
And we could do:
for _ in teams:
instead of
for i in range(len(teams))
(Unfortunately, this can hardly be adapted to the "even" situation)
Note: "_" is a usual variable names for values one does not plan to use.
Dict comprehension
The dictionnary initiation you perform via dict[index] = value
in a loop could be done using the Dictionnary Comprehension syntactic sugar.
Instead of:
dic =
for i, team in enumerate(teams):
dic[i] = team
we you can write:
dic = i: team for i, team in enumerate(teams)
Now it is much more obvious, it also corresponds to:
dic = dict(enumerate(teams))
Finally, we can ask ourselves how we use this dictionnary: the answer is "to get the team at a given index". Do we really need a dictionnay for this ? I do not think so. We can get rid of the dic
variable and use teams
directly.
At this stage, we have:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
The right tool for the task
The part:
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
could/should probably be written with pop:
arr2.pop(0)
arr1.pop()
And now, these line can be merged with arrXX.append(arrYYY[ZZ])
:
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
games.append(list(zip(arr1, arr2)))
Removing useless steps
A loop is used to fill an array. Another one is used to iterate over the array. We could try to use a single loop to do everything (disclaimer: this is not always a good idea as far as readability goes).
This removes the need for a few calls to list
.
At this stage, we have:
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
if len(teams)+1 not in gm:
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
Better indices
You generate a list of indices using i+1
and then use val - 1
when you use them. You can make your life easier twice.
Iterable unpacking
Instead of using indices to get elements from an iterable with a know number of elements, you can use iterable unpacking.
You'd get
def simulate_even_draw(teams):
"""Return the list of games."""
half_len = int(len(teams)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams))][::-1]
matches = []
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
matches.append((teams[a], teams[b]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
half_len = int((len(teams)+1)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams)+1)][::-1]
matches = []
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
if len(teams) not in (a, b):
matches.append((teams[a], teams[b]))
return matches
True divisions
Instead of using "/" and convert the float result to int, you can use "//" which is an integer division.
Other way to compute indices
We could write something like:
indices = list(range(len(teams)))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
and
indices = list(range(len(teams)+1))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
Altough, if we don't care about order, we could use the more direct:
arr1 = indices[:half_len]
arr2 = indices[half_len:]
Remove the duplicated logic
Don't repeat yourself is a principle of software development that you could easily apply here. Indeed, we have 2 functions that look very similar.
This is trickier than expected and I have to go. I may continue another day.
Batteries included
The Python standard library contains many useful things. Among them, we have the very interesting module itertools
which itself contains combinations which is what you want.
$endgroup$
add a comment |
$begingroup$
Making the code testable and tested
The first step to improve your code is to try to make it testable. By doing so, you usually have to deal with Separation of Concerns: in your case, you have to split the logic doing the output from the logic computing games. The easiest way to do so it to rewrite slightly the simulate_XXX
functions to return values instead of writing them.
Once it it done, you can easily write tests for the function computing the games (in order to make this easier to implement, I've extracted out the randomising part as well).
At this stage, we have something like:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
Now we can start improving the code in a safer way.
Remove what's not required
dic[i+1] = ''
is not required, we can remove it.
Also, resetting zipped
to the empty string is not required, we can remove it. Maybe we could get rid of zipped
altogether.
Finally, we call for gm in list(game)
when game
is already a list. We can remove the call to list
.
Loop like a native
I highly recommend Ned Batchelder's talk "Loop like a native" about iterators. One of the most simple take away is that whenever you're doing range(len(iterable)), you can probably do things in a better way: more concise, clearer and more efficient.
In your case, we could have:
for i in range(len(teams)):
dic[i] = teams[i]
replaced by
for i, team in enumerate(teams):
dic[i] = team
And we could do:
for _ in teams:
instead of
for i in range(len(teams))
(Unfortunately, this can hardly be adapted to the "even" situation)
Note: "_" is a usual variable names for values one does not plan to use.
Dict comprehension
The dictionnary initiation you perform via dict[index] = value
in a loop could be done using the Dictionnary Comprehension syntactic sugar.
Instead of:
dic =
for i, team in enumerate(teams):
dic[i] = team
we you can write:
dic = i: team for i, team in enumerate(teams)
Now it is much more obvious, it also corresponds to:
dic = dict(enumerate(teams))
Finally, we can ask ourselves how we use this dictionnary: the answer is "to get the team at a given index". Do we really need a dictionnay for this ? I do not think so. We can get rid of the dic
variable and use teams
directly.
At this stage, we have:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
The right tool for the task
The part:
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
could/should probably be written with pop:
arr2.pop(0)
arr1.pop()
And now, these line can be merged with arrXX.append(arrYYY[ZZ])
:
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
games.append(list(zip(arr1, arr2)))
Removing useless steps
A loop is used to fill an array. Another one is used to iterate over the array. We could try to use a single loop to do everything (disclaimer: this is not always a good idea as far as readability goes).
This removes the need for a few calls to list
.
At this stage, we have:
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
if len(teams)+1 not in gm:
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
Better indices
You generate a list of indices using i+1
and then use val - 1
when you use them. You can make your life easier twice.
Iterable unpacking
Instead of using indices to get elements from an iterable with a know number of elements, you can use iterable unpacking.
You'd get
def simulate_even_draw(teams):
"""Return the list of games."""
half_len = int(len(teams)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams))][::-1]
matches = []
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
matches.append((teams[a], teams[b]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
half_len = int((len(teams)+1)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams)+1)][::-1]
matches = []
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
if len(teams) not in (a, b):
matches.append((teams[a], teams[b]))
return matches
True divisions
Instead of using "/" and convert the float result to int, you can use "//" which is an integer division.
Other way to compute indices
We could write something like:
indices = list(range(len(teams)))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
and
indices = list(range(len(teams)+1))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
Altough, if we don't care about order, we could use the more direct:
arr1 = indices[:half_len]
arr2 = indices[half_len:]
Remove the duplicated logic
Don't repeat yourself is a principle of software development that you could easily apply here. Indeed, we have 2 functions that look very similar.
This is trickier than expected and I have to go. I may continue another day.
Batteries included
The Python standard library contains many useful things. Among them, we have the very interesting module itertools
which itself contains combinations which is what you want.
$endgroup$
Making the code testable and tested
The first step to improve your code is to try to make it testable. By doing so, you usually have to deal with Separation of Concerns: in your case, you have to split the logic doing the output from the logic computing games. The easiest way to do so it to rewrite slightly the simulate_XXX
functions to return values instead of writing them.
Once it it done, you can easily write tests for the function computing the games (in order to make this easier to implement, I've extracted out the randomising part as well).
At this stage, we have something like:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
games = []
arr1 = [i+1 for i in range(int(len(teams)/2))]
arr2 = [i+1 for i in range(int(len(teams)/2), len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
dic =
for i in range(len(teams)):
dic[i] = teams[i]
dic[i+1] = ''
games = []
arr1 = [i+1 for i in range(int((len(teams)+1)/2))]
arr2 = [i+1 for i in range(int((len(teams)+1)/2), len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
zipped = list(zip(arr1, arr2))
games.append(zipped)
zipped = []
for game in games:
for gm in list(game):
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = dic[r[0]-1], dic[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
Now we can start improving the code in a safer way.
Remove what's not required
dic[i+1] = ''
is not required, we can remove it.
Also, resetting zipped
to the empty string is not required, we can remove it. Maybe we could get rid of zipped
altogether.
Finally, we call for gm in list(game)
when game
is already a list. We can remove the call to list
.
Loop like a native
I highly recommend Ned Batchelder's talk "Loop like a native" about iterators. One of the most simple take away is that whenever you're doing range(len(iterable)), you can probably do things in a better way: more concise, clearer and more efficient.
In your case, we could have:
for i in range(len(teams)):
dic[i] = teams[i]
replaced by
for i, team in enumerate(teams):
dic[i] = team
And we could do:
for _ in teams:
instead of
for i in range(len(teams))
(Unfortunately, this can hardly be adapted to the "even" situation)
Note: "_" is a usual variable names for values one does not plan to use.
Dict comprehension
The dictionnary initiation you perform via dict[index] = value
in a loop could be done using the Dictionnary Comprehension syntactic sugar.
Instead of:
dic =
for i, team in enumerate(teams):
dic[i] = team
we you can write:
dic = i: team for i, team in enumerate(teams)
Now it is much more obvious, it also corresponds to:
dic = dict(enumerate(teams))
Finally, we can ask ourselves how we use this dictionnary: the answer is "to get the team at a given index". Do we really need a dictionnay for this ? I do not think so. We can get rid of the dic
variable and use teams
directly.
At this stage, we have:
import random
def simulate_draw(teams):
"""Return the list of games."""
if len(teams) % 2 == 0:
return simulate_even_draw(teams)
else:
return simulate_odd_draw(teams)
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
games = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2[0])
arr2.append(arr1[-1])
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
games.append(list(zip(arr1, arr2)))
for game in games:
for gm in game:
r = gm # remove randomness for now - random.sample(gm, len(gm))
if len(teams)+1 not in r:
a, b = teams[r[0]-1], teams[r[1]-1]
matches.append((a, b))
# print(a + ' plays ' + b)
return matches
def displays_simulated_draws(teams):
"""Print the list of games."""
for gm in simulate_draw(teams):
a, b = random.sample(gm, len(gm))
print(a + ' plays ' + b)
def test_simulate_draw():
"""Small tests for simulate_draw."""
# TODO: Use a proper testing framework
TESTS = [
([], []),
(['A'], []),
(['A', 'B', 'C', 'D'], [('A', 'C'), ('D', 'B'), ('A', 'B'), ('C', 'D'), ('A', 'D'), ('B', 'C')]),
(['A', 'B', 'C', 'D', 'E'], [('A', 'E'), ('B', 'C'), ('A', 'D'), ('E', 'C'), ('A', 'C'), ('D', 'B'), ('A', 'B'), ('D', 'E'), ('B', 'E'), ('C', 'D')]),
]
for teams, expected_out in TESTS:
# print(teams)
ret = simulate_draw(teams)
assert ret == expected_out
if __name__ == '__main__':
test_simulate_draw()
displays_simulated_draws(['A', 'B', 'C', 'D'])
The right tool for the task
The part:
arr2.remove(arr2[0])
arr1.remove(arr1[-1])
could/should probably be written with pop:
arr2.pop(0)
arr1.pop()
And now, these line can be merged with arrXX.append(arrYYY[ZZ])
:
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
games.append(list(zip(arr1, arr2)))
Removing useless steps
A loop is used to fill an array. Another one is used to iterate over the array. We could try to use a single loop to do everything (disclaimer: this is not always a good idea as far as readability goes).
This removes the need for a few calls to list
.
At this stage, we have:
def simulate_even_draw(teams):
"""Return the list of games."""
matches = []
half_len = int(len(teams)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams))][::-1]
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
matches = []
half_len = int((len(teams)+1)/2)
arr1 = [i+1 for i in range(half_len)]
arr2 = [i+1 for i in range(half_len, len(teams)+1)][::-1]
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for gm in zip(arr1, arr2):
if len(teams)+1 not in gm:
matches.append((teams[gm[0]-1], teams[gm[1]-1]))
return matches
Better indices
You generate a list of indices using i+1
and then use val - 1
when you use them. You can make your life easier twice.
Iterable unpacking
Instead of using indices to get elements from an iterable with a know number of elements, you can use iterable unpacking.
You'd get
def simulate_even_draw(teams):
"""Return the list of games."""
half_len = int(len(teams)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams))][::-1]
matches = []
for i in range(len(teams)-1):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
matches.append((teams[a], teams[b]))
return matches
def simulate_odd_draw(teams):
"""Return the list of games."""
half_len = int((len(teams)+1)/2)
arr1 = [i for i in range(half_len)]
arr2 = [i for i in range(half_len, len(teams)+1)][::-1]
matches = []
for i in range(len(teams)):
arr1.insert(1, arr2.pop(0))
arr2.append(arr1.pop())
for a, b in zip(arr1, arr2):
if len(teams) not in (a, b):
matches.append((teams[a], teams[b]))
return matches
True divisions
Instead of using "/" and convert the float result to int, you can use "//" which is an integer division.
Other way to compute indices
We could write something like:
indices = list(range(len(teams)))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
and
indices = list(range(len(teams)+1))
half_len = len(indices)//2
arr1 = indices[:half_len]
arr2 = indices[:half_len-1:-1]
Altough, if we don't care about order, we could use the more direct:
arr1 = indices[:half_len]
arr2 = indices[half_len:]
Remove the duplicated logic
Don't repeat yourself is a principle of software development that you could easily apply here. Indeed, we have 2 functions that look very similar.
This is trickier than expected and I have to go. I may continue another day.
Batteries included
The Python standard library contains many useful things. Among them, we have the very interesting module itertools
which itself contains combinations which is what you want.
edited 3 hours ago
answered 6 hours ago
JosayJosay
26.2k14087
26.2k14087
add a comment |
add a comment |
Eldar is a new contributor. Be nice, and check out our Code of Conduct.
Eldar is a new contributor. Be nice, and check out our Code of Conduct.
Eldar is a new contributor. Be nice, and check out our Code of Conduct.
Eldar is a new contributor. Be nice, and check out our Code of Conduct.
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217969%2fsimulate-round-robin-tournament-draw%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
I think your code is missing an entry point (
if __name__ == "__main__":
) because it shouldn't run in it's current format. Can you fix? Verify by copying then pasting into a new file and running that.$endgroup$
– C. Harley
8 hours ago
$begingroup$
@C.Harley This seems to be a lib, as such, I don't see the necessity for an entry point. You can call
simulate_draw
with a list of names of your liking if you want to test it.$endgroup$
– Mathias Ettinger
6 hours ago