DM::OJ Slot Machines challenge in Python 3
$begingroup$
I have written a solution to the Slot Machines challenge on DM::OJ:
Martha takes a jar of quarters to the casino with the intention of becoming rich. She plays three machines in turn. Unknown to her, the machines are entirely predictable. Each play costs one quarter. The first machine pays 30 quarters every 35th time it is played; the second machine pays 60 quarters every 100th time it is played; the third pays
9 quarters every 10th time it is played.
Input Specification
Your program should take as input the number of quarters in Martha's jar (there will be at least one and fewer than 1000), and the number of times each machine has been played since it last paid.
Output Specification
Your program should output the number of times Martha plays until she goes broke.
Sample Input
48
3
10
4
Sample Output
Martha plays 66 times before going broke.
I am seeking to make it run in 2 seconds or less.
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
turns = 0
while (money != 0):
#machine 1
money -= 1
m1 += 1
if (m1 == 35):
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if (m2 == 100):
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if (m3 == 10):
money += 9
m3 = 0
turns += 1
print ('Martha plays {} times before going broke.'.format(turns))
python performance python-3.x programming-challenge simulation
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
add a comment |
$begingroup$
I have written a solution to the Slot Machines challenge on DM::OJ:
Martha takes a jar of quarters to the casino with the intention of becoming rich. She plays three machines in turn. Unknown to her, the machines are entirely predictable. Each play costs one quarter. The first machine pays 30 quarters every 35th time it is played; the second machine pays 60 quarters every 100th time it is played; the third pays
9 quarters every 10th time it is played.
Input Specification
Your program should take as input the number of quarters in Martha's jar (there will be at least one and fewer than 1000), and the number of times each machine has been played since it last paid.
Output Specification
Your program should output the number of times Martha plays until she goes broke.
Sample Input
48
3
10
4
Sample Output
Martha plays 66 times before going broke.
I am seeking to make it run in 2 seconds or less.
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
turns = 0
while (money != 0):
#machine 1
money -= 1
m1 += 1
if (m1 == 35):
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if (m2 == 100):
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if (m3 == 10):
money += 9
m3 = 0
turns += 1
print ('Martha plays {} times before going broke.'.format(turns))
python performance python-3.x programming-challenge simulation
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
add a comment |
$begingroup$
I have written a solution to the Slot Machines challenge on DM::OJ:
Martha takes a jar of quarters to the casino with the intention of becoming rich. She plays three machines in turn. Unknown to her, the machines are entirely predictable. Each play costs one quarter. The first machine pays 30 quarters every 35th time it is played; the second machine pays 60 quarters every 100th time it is played; the third pays
9 quarters every 10th time it is played.
Input Specification
Your program should take as input the number of quarters in Martha's jar (there will be at least one and fewer than 1000), and the number of times each machine has been played since it last paid.
Output Specification
Your program should output the number of times Martha plays until she goes broke.
Sample Input
48
3
10
4
Sample Output
Martha plays 66 times before going broke.
I am seeking to make it run in 2 seconds or less.
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
turns = 0
while (money != 0):
#machine 1
money -= 1
m1 += 1
if (m1 == 35):
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if (m2 == 100):
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if (m3 == 10):
money += 9
m3 = 0
turns += 1
print ('Martha plays {} times before going broke.'.format(turns))
python performance python-3.x programming-challenge simulation
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
$endgroup$
I have written a solution to the Slot Machines challenge on DM::OJ:
Martha takes a jar of quarters to the casino with the intention of becoming rich. She plays three machines in turn. Unknown to her, the machines are entirely predictable. Each play costs one quarter. The first machine pays 30 quarters every 35th time it is played; the second machine pays 60 quarters every 100th time it is played; the third pays
9 quarters every 10th time it is played.
Input Specification
Your program should take as input the number of quarters in Martha's jar (there will be at least one and fewer than 1000), and the number of times each machine has been played since it last paid.
Output Specification
Your program should output the number of times Martha plays until she goes broke.
Sample Input
48
3
10
4
Sample Output
Martha plays 66 times before going broke.
I am seeking to make it run in 2 seconds or less.
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
turns = 0
while (money != 0):
#machine 1
money -= 1
m1 += 1
if (m1 == 35):
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if (m2 == 100):
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if (m3 == 10):
money += 9
m3 = 0
turns += 1
print ('Martha plays {} times before going broke.'.format(turns))
python performance python-3.x programming-challenge simulation
python performance python-3.x programming-challenge simulation
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
edited 7 hours ago
200_success
129k15152415
129k15152415
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
asked 9 hours ago
Arihan SharmaArihan Sharma
84
84
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
New contributor
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
Arihan Sharma is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
add a comment |
add a comment |
1 Answer
1
active
oldest
votes
$begingroup$
Code organisation, separation of concerns and tests
The code is a single monolithic piece of code. It could be a good idea to reorganise things a bit: to make it clearer, to make it easier to test, to make it easier to maintain, etc.
The first major improvement could be write a function with a clear input (money, m1, m2, m3) and a clear output solving the issue we care about: computing the number of turns.
The logic handling the input/output is separated from the logic performing the computation - seel also https://en.wikipedia.org/wiki/Separation_of_concerns .
Then, this function could be used with inputs from the user but we could also feed it hardcoded values. This can be used to write tests to ensure that the function behaves properly (at least on the example provided by dmoj).
(In the code provided below, I've a simple assert statement to write the tests but it could be a good chance to read about unit test frameworks and start to use them).
Then, you can add any number of tests easily.
Now that we have unit-tests to prevent us from breaking things too badly, we can start tweaking the code.
Finally, you could use if __name__ == "__main__": to separate the definitions in your code from the code actually using these definitions. It makes your code easier to reuse via the import mechanism.
Style
Python has a Style Guide called PEP 8. It is followed (more or less stricly) by the Python community. It is highly recommended to read it and try to stick to it when relevant. In your case, you have superfluous parenthesis, indentations of 2 spaces instead of 4, superfluous whitespaces.
At this stage, we have:
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
#machine 1
money -= 1
m1 += 1
if m1 == 35:
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == 100:
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == 10:
money += 9
m3 = 0
turns += 1
return turns
def interactive_main():
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
print('Martha plays {} times before going broke.'.format(get_number_of_games(money, m1, m2, m3)))
def unit_test():
"""Run unit-tests."""
assert get_number_of_games(48, 3, 10, 4) == 66
print("OK")
if __name__ == "__main__":
unit_test()
Magic numbers
The code is full of magic numbers, making the logic hard to understand.
We could store these values into constant with a meaningful name.
Even better, we could store these into appropriate data structures. For a simple solutions, we could decide to use namedtuples.
from collections import namedtuple
Machine = namedtuple('Machine', ['winning_freq', 'profit'])
MACHINE1 = Machine(35, 30)
MACHINE2 = Machine(100, 60)
MACHINE3 = Machine(10, 9)
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
money -= 1
m1 += 1
if m1 == MACHINE1.winning_freq:
money += MACHINE1.profit
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == MACHINE2.winning_freq:
money += MACHINE2.profit
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == MACHINE3.winning_freq:
money += MACHINE3.profit
m3 = 0
turns += 1
return turns
Un-handled situations (also known as bugs)
The code is not very robust. A few situations can lead it astray.
Let's take a simple example: we have 1 quarter and the machines haven't been used. This can be written as:
print(get_number_of_games(1, 0, 0, 0))
When we run this, we are stuck into a infinite loop. I'll let you investigate the reason why.
Another example - which could be considered invalid but is definitly worth fixing because of how simple it is: we have a lot of money and the machines have been used a lot. For instance, we have:
print(get_number_of_games(1000, 36, 101, 11))
Assuming we fix the previous issue: how many turns do we play ? How many times have we won ?
As we fix behaviors, you can add corresponding unit-tests to be sure you don't fall into the same issue later on.
Going further
The code relies on duplicated logic to handle the different machines. A better solution could be to handle them via the same logic. We could imagine having a list of machines.
Also, we could imagine having the numbers of turns played on a given machine stored along with the other information in a Machine class. Then, the get_number_of_games functions could take as inputs an amount of money and a list of properly defined Machines.
Take-away for Python and non-Python code
- Make your code testable
- Make your code tested and correct
- Improve it while keeping properties 1 and 2.
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
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
});
}
});
Arihan Sharma 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%2f211709%2fdmoj-slot-machines-challenge-in-python-3%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$
Code organisation, separation of concerns and tests
The code is a single monolithic piece of code. It could be a good idea to reorganise things a bit: to make it clearer, to make it easier to test, to make it easier to maintain, etc.
The first major improvement could be write a function with a clear input (money, m1, m2, m3) and a clear output solving the issue we care about: computing the number of turns.
The logic handling the input/output is separated from the logic performing the computation - seel also https://en.wikipedia.org/wiki/Separation_of_concerns .
Then, this function could be used with inputs from the user but we could also feed it hardcoded values. This can be used to write tests to ensure that the function behaves properly (at least on the example provided by dmoj).
(In the code provided below, I've a simple assert statement to write the tests but it could be a good chance to read about unit test frameworks and start to use them).
Then, you can add any number of tests easily.
Now that we have unit-tests to prevent us from breaking things too badly, we can start tweaking the code.
Finally, you could use if __name__ == "__main__": to separate the definitions in your code from the code actually using these definitions. It makes your code easier to reuse via the import mechanism.
Style
Python has a Style Guide called PEP 8. It is followed (more or less stricly) by the Python community. It is highly recommended to read it and try to stick to it when relevant. In your case, you have superfluous parenthesis, indentations of 2 spaces instead of 4, superfluous whitespaces.
At this stage, we have:
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
#machine 1
money -= 1
m1 += 1
if m1 == 35:
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == 100:
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == 10:
money += 9
m3 = 0
turns += 1
return turns
def interactive_main():
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
print('Martha plays {} times before going broke.'.format(get_number_of_games(money, m1, m2, m3)))
def unit_test():
"""Run unit-tests."""
assert get_number_of_games(48, 3, 10, 4) == 66
print("OK")
if __name__ == "__main__":
unit_test()
Magic numbers
The code is full of magic numbers, making the logic hard to understand.
We could store these values into constant with a meaningful name.
Even better, we could store these into appropriate data structures. For a simple solutions, we could decide to use namedtuples.
from collections import namedtuple
Machine = namedtuple('Machine', ['winning_freq', 'profit'])
MACHINE1 = Machine(35, 30)
MACHINE2 = Machine(100, 60)
MACHINE3 = Machine(10, 9)
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
money -= 1
m1 += 1
if m1 == MACHINE1.winning_freq:
money += MACHINE1.profit
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == MACHINE2.winning_freq:
money += MACHINE2.profit
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == MACHINE3.winning_freq:
money += MACHINE3.profit
m3 = 0
turns += 1
return turns
Un-handled situations (also known as bugs)
The code is not very robust. A few situations can lead it astray.
Let's take a simple example: we have 1 quarter and the machines haven't been used. This can be written as:
print(get_number_of_games(1, 0, 0, 0))
When we run this, we are stuck into a infinite loop. I'll let you investigate the reason why.
Another example - which could be considered invalid but is definitly worth fixing because of how simple it is: we have a lot of money and the machines have been used a lot. For instance, we have:
print(get_number_of_games(1000, 36, 101, 11))
Assuming we fix the previous issue: how many turns do we play ? How many times have we won ?
As we fix behaviors, you can add corresponding unit-tests to be sure you don't fall into the same issue later on.
Going further
The code relies on duplicated logic to handle the different machines. A better solution could be to handle them via the same logic. We could imagine having a list of machines.
Also, we could imagine having the numbers of turns played on a given machine stored along with the other information in a Machine class. Then, the get_number_of_games functions could take as inputs an amount of money and a list of properly defined Machines.
Take-away for Python and non-Python code
- Make your code testable
- Make your code tested and correct
- Improve it while keeping properties 1 and 2.
$endgroup$
add a comment |
$begingroup$
Code organisation, separation of concerns and tests
The code is a single monolithic piece of code. It could be a good idea to reorganise things a bit: to make it clearer, to make it easier to test, to make it easier to maintain, etc.
The first major improvement could be write a function with a clear input (money, m1, m2, m3) and a clear output solving the issue we care about: computing the number of turns.
The logic handling the input/output is separated from the logic performing the computation - seel also https://en.wikipedia.org/wiki/Separation_of_concerns .
Then, this function could be used with inputs from the user but we could also feed it hardcoded values. This can be used to write tests to ensure that the function behaves properly (at least on the example provided by dmoj).
(In the code provided below, I've a simple assert statement to write the tests but it could be a good chance to read about unit test frameworks and start to use them).
Then, you can add any number of tests easily.
Now that we have unit-tests to prevent us from breaking things too badly, we can start tweaking the code.
Finally, you could use if __name__ == "__main__": to separate the definitions in your code from the code actually using these definitions. It makes your code easier to reuse via the import mechanism.
Style
Python has a Style Guide called PEP 8. It is followed (more or less stricly) by the Python community. It is highly recommended to read it and try to stick to it when relevant. In your case, you have superfluous parenthesis, indentations of 2 spaces instead of 4, superfluous whitespaces.
At this stage, we have:
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
#machine 1
money -= 1
m1 += 1
if m1 == 35:
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == 100:
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == 10:
money += 9
m3 = 0
turns += 1
return turns
def interactive_main():
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
print('Martha plays {} times before going broke.'.format(get_number_of_games(money, m1, m2, m3)))
def unit_test():
"""Run unit-tests."""
assert get_number_of_games(48, 3, 10, 4) == 66
print("OK")
if __name__ == "__main__":
unit_test()
Magic numbers
The code is full of magic numbers, making the logic hard to understand.
We could store these values into constant with a meaningful name.
Even better, we could store these into appropriate data structures. For a simple solutions, we could decide to use namedtuples.
from collections import namedtuple
Machine = namedtuple('Machine', ['winning_freq', 'profit'])
MACHINE1 = Machine(35, 30)
MACHINE2 = Machine(100, 60)
MACHINE3 = Machine(10, 9)
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
money -= 1
m1 += 1
if m1 == MACHINE1.winning_freq:
money += MACHINE1.profit
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == MACHINE2.winning_freq:
money += MACHINE2.profit
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == MACHINE3.winning_freq:
money += MACHINE3.profit
m3 = 0
turns += 1
return turns
Un-handled situations (also known as bugs)
The code is not very robust. A few situations can lead it astray.
Let's take a simple example: we have 1 quarter and the machines haven't been used. This can be written as:
print(get_number_of_games(1, 0, 0, 0))
When we run this, we are stuck into a infinite loop. I'll let you investigate the reason why.
Another example - which could be considered invalid but is definitly worth fixing because of how simple it is: we have a lot of money and the machines have been used a lot. For instance, we have:
print(get_number_of_games(1000, 36, 101, 11))
Assuming we fix the previous issue: how many turns do we play ? How many times have we won ?
As we fix behaviors, you can add corresponding unit-tests to be sure you don't fall into the same issue later on.
Going further
The code relies on duplicated logic to handle the different machines. A better solution could be to handle them via the same logic. We could imagine having a list of machines.
Also, we could imagine having the numbers of turns played on a given machine stored along with the other information in a Machine class. Then, the get_number_of_games functions could take as inputs an amount of money and a list of properly defined Machines.
Take-away for Python and non-Python code
- Make your code testable
- Make your code tested and correct
- Improve it while keeping properties 1 and 2.
$endgroup$
add a comment |
$begingroup$
Code organisation, separation of concerns and tests
The code is a single monolithic piece of code. It could be a good idea to reorganise things a bit: to make it clearer, to make it easier to test, to make it easier to maintain, etc.
The first major improvement could be write a function with a clear input (money, m1, m2, m3) and a clear output solving the issue we care about: computing the number of turns.
The logic handling the input/output is separated from the logic performing the computation - seel also https://en.wikipedia.org/wiki/Separation_of_concerns .
Then, this function could be used with inputs from the user but we could also feed it hardcoded values. This can be used to write tests to ensure that the function behaves properly (at least on the example provided by dmoj).
(In the code provided below, I've a simple assert statement to write the tests but it could be a good chance to read about unit test frameworks and start to use them).
Then, you can add any number of tests easily.
Now that we have unit-tests to prevent us from breaking things too badly, we can start tweaking the code.
Finally, you could use if __name__ == "__main__": to separate the definitions in your code from the code actually using these definitions. It makes your code easier to reuse via the import mechanism.
Style
Python has a Style Guide called PEP 8. It is followed (more or less stricly) by the Python community. It is highly recommended to read it and try to stick to it when relevant. In your case, you have superfluous parenthesis, indentations of 2 spaces instead of 4, superfluous whitespaces.
At this stage, we have:
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
#machine 1
money -= 1
m1 += 1
if m1 == 35:
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == 100:
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == 10:
money += 9
m3 = 0
turns += 1
return turns
def interactive_main():
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
print('Martha plays {} times before going broke.'.format(get_number_of_games(money, m1, m2, m3)))
def unit_test():
"""Run unit-tests."""
assert get_number_of_games(48, 3, 10, 4) == 66
print("OK")
if __name__ == "__main__":
unit_test()
Magic numbers
The code is full of magic numbers, making the logic hard to understand.
We could store these values into constant with a meaningful name.
Even better, we could store these into appropriate data structures. For a simple solutions, we could decide to use namedtuples.
from collections import namedtuple
Machine = namedtuple('Machine', ['winning_freq', 'profit'])
MACHINE1 = Machine(35, 30)
MACHINE2 = Machine(100, 60)
MACHINE3 = Machine(10, 9)
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
money -= 1
m1 += 1
if m1 == MACHINE1.winning_freq:
money += MACHINE1.profit
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == MACHINE2.winning_freq:
money += MACHINE2.profit
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == MACHINE3.winning_freq:
money += MACHINE3.profit
m3 = 0
turns += 1
return turns
Un-handled situations (also known as bugs)
The code is not very robust. A few situations can lead it astray.
Let's take a simple example: we have 1 quarter and the machines haven't been used. This can be written as:
print(get_number_of_games(1, 0, 0, 0))
When we run this, we are stuck into a infinite loop. I'll let you investigate the reason why.
Another example - which could be considered invalid but is definitly worth fixing because of how simple it is: we have a lot of money and the machines have been used a lot. For instance, we have:
print(get_number_of_games(1000, 36, 101, 11))
Assuming we fix the previous issue: how many turns do we play ? How many times have we won ?
As we fix behaviors, you can add corresponding unit-tests to be sure you don't fall into the same issue later on.
Going further
The code relies on duplicated logic to handle the different machines. A better solution could be to handle them via the same logic. We could imagine having a list of machines.
Also, we could imagine having the numbers of turns played on a given machine stored along with the other information in a Machine class. Then, the get_number_of_games functions could take as inputs an amount of money and a list of properly defined Machines.
Take-away for Python and non-Python code
- Make your code testable
- Make your code tested and correct
- Improve it while keeping properties 1 and 2.
$endgroup$
Code organisation, separation of concerns and tests
The code is a single monolithic piece of code. It could be a good idea to reorganise things a bit: to make it clearer, to make it easier to test, to make it easier to maintain, etc.
The first major improvement could be write a function with a clear input (money, m1, m2, m3) and a clear output solving the issue we care about: computing the number of turns.
The logic handling the input/output is separated from the logic performing the computation - seel also https://en.wikipedia.org/wiki/Separation_of_concerns .
Then, this function could be used with inputs from the user but we could also feed it hardcoded values. This can be used to write tests to ensure that the function behaves properly (at least on the example provided by dmoj).
(In the code provided below, I've a simple assert statement to write the tests but it could be a good chance to read about unit test frameworks and start to use them).
Then, you can add any number of tests easily.
Now that we have unit-tests to prevent us from breaking things too badly, we can start tweaking the code.
Finally, you could use if __name__ == "__main__": to separate the definitions in your code from the code actually using these definitions. It makes your code easier to reuse via the import mechanism.
Style
Python has a Style Guide called PEP 8. It is followed (more or less stricly) by the Python community. It is highly recommended to read it and try to stick to it when relevant. In your case, you have superfluous parenthesis, indentations of 2 spaces instead of 4, superfluous whitespaces.
At this stage, we have:
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
#machine 1
money -= 1
m1 += 1
if m1 == 35:
money += 30
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == 100:
money += 60
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == 10:
money += 9
m3 = 0
turns += 1
return turns
def interactive_main():
money = int(input())
m1 = int(input())
m2 = int(input())
m3 = int(input())
print('Martha plays {} times before going broke.'.format(get_number_of_games(money, m1, m2, m3)))
def unit_test():
"""Run unit-tests."""
assert get_number_of_games(48, 3, 10, 4) == 66
print("OK")
if __name__ == "__main__":
unit_test()
Magic numbers
The code is full of magic numbers, making the logic hard to understand.
We could store these values into constant with a meaningful name.
Even better, we could store these into appropriate data structures. For a simple solutions, we could decide to use namedtuples.
from collections import namedtuple
Machine = namedtuple('Machine', ['winning_freq', 'profit'])
MACHINE1 = Machine(35, 30)
MACHINE2 = Machine(100, 60)
MACHINE3 = Machine(10, 9)
def get_number_of_games(money, m1, m2, m3):
"""Return the number of games <to be continued>."""
turns = 0
while money != 0:
money -= 1
m1 += 1
if m1 == MACHINE1.winning_freq:
money += MACHINE1.profit
m1 = 0
turns += 1
#machine 2
money -= 1
m2 += 1
if m2 == MACHINE2.winning_freq:
money += MACHINE2.profit
m2 = 0
turns += 1
#machine 3
money -= 1
m3 += 1
if m3 == MACHINE3.winning_freq:
money += MACHINE3.profit
m3 = 0
turns += 1
return turns
Un-handled situations (also known as bugs)
The code is not very robust. A few situations can lead it astray.
Let's take a simple example: we have 1 quarter and the machines haven't been used. This can be written as:
print(get_number_of_games(1, 0, 0, 0))
When we run this, we are stuck into a infinite loop. I'll let you investigate the reason why.
Another example - which could be considered invalid but is definitly worth fixing because of how simple it is: we have a lot of money and the machines have been used a lot. For instance, we have:
print(get_number_of_games(1000, 36, 101, 11))
Assuming we fix the previous issue: how many turns do we play ? How many times have we won ?
As we fix behaviors, you can add corresponding unit-tests to be sure you don't fall into the same issue later on.
Going further
The code relies on duplicated logic to handle the different machines. A better solution could be to handle them via the same logic. We could imagine having a list of machines.
Also, we could imagine having the numbers of turns played on a given machine stored along with the other information in a Machine class. Then, the get_number_of_games functions could take as inputs an amount of money and a list of properly defined Machines.
Take-away for Python and non-Python code
- Make your code testable
- Make your code tested and correct
- Improve it while keeping properties 1 and 2.
answered 8 hours ago
JosayJosay
25.7k14087
25.7k14087
add a comment |
add a comment |
Arihan Sharma is a new contributor. Be nice, and check out our Code of Conduct.
Arihan Sharma is a new contributor. Be nice, and check out our Code of Conduct.
Arihan Sharma is a new contributor. Be nice, and check out our Code of Conduct.
Arihan Sharma 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%2f211709%2fdmoj-slot-machines-challenge-in-python-3%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