Expired---------------












-1












$begingroup$


This post has been deleted #001010101010100200



print("This code is expired")
print("See receipt #001010101010100200
print("Completed")









share|improve this question











$endgroup$








  • 1




    $begingroup$
    (My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
    $endgroup$
    – greybeard
    Feb 4 at 23:36










  • $begingroup$
    Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 4 at 23:36










  • $begingroup$
    Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
    $endgroup$
    – Josay
    Feb 5 at 0:15












  • $begingroup$
    Actually, the code may be properly formated but is just confusing.
    $endgroup$
    – Josay
    Feb 5 at 0:16










  • $begingroup$
    @Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 5 at 0:36


















-1












$begingroup$


This post has been deleted #001010101010100200



print("This code is expired")
print("See receipt #001010101010100200
print("Completed")









share|improve this question











$endgroup$








  • 1




    $begingroup$
    (My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
    $endgroup$
    – greybeard
    Feb 4 at 23:36










  • $begingroup$
    Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 4 at 23:36










  • $begingroup$
    Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
    $endgroup$
    – Josay
    Feb 5 at 0:15












  • $begingroup$
    Actually, the code may be properly formated but is just confusing.
    $endgroup$
    – Josay
    Feb 5 at 0:16










  • $begingroup$
    @Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 5 at 0:36
















-1












-1








-1





$begingroup$


This post has been deleted #001010101010100200



print("This code is expired")
print("See receipt #001010101010100200
print("Completed")









share|improve this question











$endgroup$




This post has been deleted #001010101010100200



print("This code is expired")
print("See receipt #001010101010100200
print("Completed")






python-3.x






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited 20 mins ago







ThatNewbieCoder101

















asked Feb 4 at 23:23









ThatNewbieCoder101ThatNewbieCoder101

12




12








  • 1




    $begingroup$
    (My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
    $endgroup$
    – greybeard
    Feb 4 at 23:36










  • $begingroup$
    Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 4 at 23:36










  • $begingroup$
    Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
    $endgroup$
    – Josay
    Feb 5 at 0:15












  • $begingroup$
    Actually, the code may be properly formated but is just confusing.
    $endgroup$
    – Josay
    Feb 5 at 0:16










  • $begingroup$
    @Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 5 at 0:36
















  • 1




    $begingroup$
    (My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
    $endgroup$
    – greybeard
    Feb 4 at 23:36










  • $begingroup$
    Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 4 at 23:36










  • $begingroup$
    Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
    $endgroup$
    – Josay
    Feb 5 at 0:15












  • $begingroup$
    Actually, the code may be properly formated but is just confusing.
    $endgroup$
    – Josay
    Feb 5 at 0:16










  • $begingroup$
    @Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
    $endgroup$
    – Sᴀᴍ Onᴇᴌᴀ
    Feb 5 at 0:36










1




1




$begingroup$
(My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
$endgroup$
– greybeard
Feb 4 at 23:36




$begingroup$
(My pet peeve: you present uncommented/undocumented code.) I'm experiencing some issues please describe these, and why you have trouble resolving them.
$endgroup$
– greybeard
Feb 4 at 23:36












$begingroup$
Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
Feb 4 at 23:36




$begingroup$
Welcome to Code Review! When adding additional information you should edit your question instead of adding a comment. I have added that information to your post. Learn more about comments including when to comment and when not to in the Help Center page about Comments.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
Feb 4 at 23:36












$begingroup$
Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
$endgroup$
– Josay
Feb 5 at 0:15






$begingroup$
Welcore here! I have the feeling the code is not formatted properly. I highly recommend using the syntax: <triple backticks><newline><your code><newline><triple backticks> to format bug chunks of code (backtick is `)
$endgroup$
– Josay
Feb 5 at 0:15














$begingroup$
Actually, the code may be properly formated but is just confusing.
$endgroup$
– Josay
Feb 5 at 0:16




$begingroup$
Actually, the code may be properly formated but is just confusing.
$endgroup$
– Josay
Feb 5 at 0:16












$begingroup$
@Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
Feb 5 at 0:36






$begingroup$
@Josay I recommend reading the Help center page for markdown formatting of code. The OP has used the standard four spaces for indentation. Code fences (as you described with three backticks) are supported as well - click the show more link to read more about that section.
$endgroup$
– Sᴀᴍ Onᴇᴌᴀ
Feb 5 at 0:36












1 Answer
1






active

oldest

votes


















4












$begingroup$

Code organisation



Having many functions defined in the middle of the core loop make things very hard to understand. It would be much easier and much more conventional to define all functions before the loop. One of the issues you might encounter is that functions will rely on variables they know nothing about such as players. A simple solution could be to add these as parameters.



Also, main is a very confusing name for a function which is not the first thing launched. I guess play_turn would be a better name.



At this stage, we'd have something like:



Disclaimer: I've changed various details (removed fstrings because I don't have Python 3.6 available, etc)



import random, time

SIMU = True # hack for testing purposes

def display_intro():
title = "** A Simple Math Quiz **"
print("*" * len(title))
print(title)
print("*" * len(title))

def display_menu():
menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
print(menu_list[0])
print(menu_list[1])
print(menu_list[2])
print(menu_list[3])
print(menu_list[4])



def display_separator():
print("-" * 24)

def get_user_input():
user_input = int(input("Enter your choice: "))
while user_input > 5 or user_input <= 0:
print("Invalid menu option.")
user_input = int(input("Please try again: "))
else:
return user_input

def check_solution(user_solution, solution, count):
if user_solution == solution:
count = count + 1
print("Correct.")
return count
else:
print("Incorrect.")
return count

def get_user_solution(problem, player):
if not SIMU:
time.sleep(random.randint(1,8))
then = time.time()
print(problem, end="")
result = int(input(" = "))
reaction_time = time.time()-then
print(" ms")
player["points"] += reaction_time
return result

def menu_option(index, count, player):
number_one = random.randrange(1, 21)
number_two = random.randrange(1, 21)
if index is 1:
problem = str(number_one) + " + " + str(number_two)
solution = number_one + number_two
user_solution = get_user_solution(problem, player)
count = check_solution(user_solution, solution, count)
return count
elif index is 2:
problem = str(number_one) + " - " + str(number_two)
solution = number_one - number_two
user_solution = get_user_solution(problem, player)
count = check_solution(user_solution, solution, count)
return count
elif index is 3:
problem = str(number_one) + " * " + str(number_two)
solution = number_one * number_two
user_solution = get_user_solution(problem, player)
count = check_solution(user_solution, solution, count)
return count
else:
problem = str(number_one) + " // " + str(number_two)
solution = number_one // number_two
user_solution = get_user_solution(problem, player)
count = check_solution(user_solution, solution, count)
return count

def display_result(total, correct):
if total > 0:
result = correct / total
percentage = round((result * 100), 2)
if total == 0:
percentage = 0
print("You answered", total, "questions with", correct, "correct.")
print("Your score is ", percentage, "%. Thank you.", sep = "")

winner = min(players,key=lambda a: a['points'])['name']
print("The winner is...")
time.sleep(0.5)
print("{winner}!")

def play_turn(player):
if SIMU:
print("It is now 's turn.nBe ready.")
else:
input("It is now 's turn.nPress enter when ready.")
display_intro()
display_menu()
display_separator()

option = get_user_input()
total = 0
correct = 0
while option != 5:
total = total + 1
correct = menu_option(option, correct, player)
option = get_user_input()

print("Exit the quiz.")
display_separator()
display_result(total, correct)

def play_round(players):
print("-"*20 + "nIt is now round !n" + "-"*20)

for player in players:
play_turn(player)

def play_game():
if SIMU:
players = [{'name': name, 'points': 0} for name in ('foo', 'bar', 'foobar')]
nb_rounds = 1
else:
players = [{
'name':input("Player: "),
'points':0
} for i in range(int(input("How many players? ")))]
nb_rounds = int(input("How many rounds? "))
for i in range(0, nb_rounds):
play_round(players)

play_game()


Now, it it easier to improve these functions.



Improve display_intro



We could compute the length and the border string in a single place:



def display_intro():
title = "** A Simple Math Quiz **"
border = "*" * len(title)
print(border)
print(title)
print(border)


Improve display_menu



It is a good idea to have the menu in a proper data structure. It would be even better too iterate over it instead of using indices: it would be more concise, more efficient and easier to maintain (you do not have to remember to add/remove lines when you add/remove elements from the menu).



def display_menu():
menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
for menu_item in menu_list:
print(menu_item)


Also, we could have the numbers generated automatically but we'll come back to this.



Improve check_solution



You have the same thing returned in both branches. You could have it in a single place:



def check_solution(user_solution, solution, count):
if user_solution == solution:
count = count + 1
print("Correct.")
else:
print("Incorrect.")
return count


Improving display_result



You could use elif at the beginning because the 2 cases are mutually exclusive:



if total > 0:
result = correct / total
percentage = round((result * 100), 2)
elif total == 0:
percentage = 0


However, I'd rather have this written:



if total == 0:
percentage = 0
else:
result = correct / total
percentage = round((result * 100), 2)


Improve menu_option



The main thing one can see is that it contains highly duplicated code.



Let's extract out the common part to make things clearer:



def menu_option(index, count, player):
number_one = random.randrange(1, 21)
number_two = random.randrange(1, 21)
if index is 1:
op_str = " + "
res = number_one + number_two
elif index is 2:
op_str = " - "
res = number_one - number_two
elif index is 3:
op_str = " * "
res = number_one * number_two
else:
op_str = " // "
res = number_one // number_two
problem = str(number_one) + op_str + str(number_two)
user_solution = get_user_solution(problem, player)
count = check_solution(user_solution, res, count)
return count


You use is to compare values. You should actually use == at that stage. You'll find more online about the is operator.



Also, instead of writing that code with multiple conditions, we could try to fit the relevant data in a proper data structure, for instance a dictionnary mapping indices to information about the operation to be performed. The operator module will be helpful here (you could also use lambda functions).



You'd have something like:



import operator
def menu_option(index, count, player):
number_one = random.randrange(1, 21)
number_two = random.randrange(1, 21)
operations = {
1: (" + ", operator.add),
2: (" - ", operator.sub),
3: (" * ", operator.mul),
4: (" // ", operator.floordiv),
}
op_str, op_func = operations[index]
res = op_func(number_one, number_two)
problem = str(number_one) + op_str + str(number_two)
count = check_solution(get_user_solution(problem, player), res, count)
return count


Now, we can think about the real meaning of the keys of that dict: they correspond to the user inputs we display in display_menu.



If we want to add or remove a supported mathematical operation, it is very hard to get it right. We should try to use a Single Source of Truth.



Then we also realise that the same information is also used in get_user_input along with the input to exit. It would make sense to get rid of that magic number which appears also in play_turn.



We could write something like:



OPERATIONS = {
1: (" + ", "Addition", operator.add),
2: (" - ", "Substraction", operator.sub),
3: (" * ", "Multiplication", operator.mul),
4: (" // ", "Integer Division", operator.floordiv),
}
EXIT_KEY = 5

def display_menu():
for k, v in OPERATIONS.items():
op_str, op_name, op_func = v
print(str(k) + ". " + op_name)
print(str(EXIT_KEY) + ". Exit")

def get_user_input():
user_input = int(input("Enter your choice: "))
while True:
if user_input in OPERATIONS or user_input == EXIT_KEY:
return user_input
print("Invalid menu option.")
user_input = int(input("Please try again: "))


Now, adding or removing an operation is much more trivial. Also, if we want to start to use letters instead of numbers for the menu options, it is quite easy to do as well.



Details



total = total + 1 can be written total += 1.



Small functions and responsabilities



Instead of having check_solution called with a count value and returning the value incremented or not, we could ensure the value just returns a boolean and the calling function is reponsible for using it.



def menu_option(index, count, player):
n1 = random.randrange(1, 21)
n2 = random.randrange(1, 21)
op_str, op_name, op_func = OPERATIONS[index]
res = op_func(n1, n2)
problem = str(n1) + op_str + str(n2)
if check_solution(get_user_solution(problem, player), res):
count += 1
return count

def check_solution(user_solution, solution):
correct = user_solution == solution
print("Correct." if correct else "Incorrect.")
return correct


Now, the same logic actually applies to menu_option:



def menu_option(index, player):
n1 = random.randrange(1, 21)
n2 = random.randrange(1, 21)
op_str, op_name, op_func = OPERATIONS[index]
res = op_func(n1, n2)
problem = str(n1) + op_str + str(n2)
return check_solution(get_user_solution(problem, player), res)

def check_solution(user_solution, solution):
correct = user_solution == solution
print("Correct." if correct else "Incorrect.")
return correct

...

while option != EXIT_KEY:
total += 1
if menu_option(option, player):
correct += 1
option = get_user_input()


At that stage, I start to feel that the function names could be improved.






share|improve this answer











$endgroup$













    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
    });


    }
    });














    draft saved

    draft discarded


















    StackExchange.ready(
    function () {
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f212884%2fexpired%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









    4












    $begingroup$

    Code organisation



    Having many functions defined in the middle of the core loop make things very hard to understand. It would be much easier and much more conventional to define all functions before the loop. One of the issues you might encounter is that functions will rely on variables they know nothing about such as players. A simple solution could be to add these as parameters.



    Also, main is a very confusing name for a function which is not the first thing launched. I guess play_turn would be a better name.



    At this stage, we'd have something like:



    Disclaimer: I've changed various details (removed fstrings because I don't have Python 3.6 available, etc)



    import random, time

    SIMU = True # hack for testing purposes

    def display_intro():
    title = "** A Simple Math Quiz **"
    print("*" * len(title))
    print(title)
    print("*" * len(title))

    def display_menu():
    menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
    print(menu_list[0])
    print(menu_list[1])
    print(menu_list[2])
    print(menu_list[3])
    print(menu_list[4])



    def display_separator():
    print("-" * 24)

    def get_user_input():
    user_input = int(input("Enter your choice: "))
    while user_input > 5 or user_input <= 0:
    print("Invalid menu option.")
    user_input = int(input("Please try again: "))
    else:
    return user_input

    def check_solution(user_solution, solution, count):
    if user_solution == solution:
    count = count + 1
    print("Correct.")
    return count
    else:
    print("Incorrect.")
    return count

    def get_user_solution(problem, player):
    if not SIMU:
    time.sleep(random.randint(1,8))
    then = time.time()
    print(problem, end="")
    result = int(input(" = "))
    reaction_time = time.time()-then
    print(" ms")
    player["points"] += reaction_time
    return result

    def menu_option(index, count, player):
    number_one = random.randrange(1, 21)
    number_two = random.randrange(1, 21)
    if index is 1:
    problem = str(number_one) + " + " + str(number_two)
    solution = number_one + number_two
    user_solution = get_user_solution(problem, player)
    count = check_solution(user_solution, solution, count)
    return count
    elif index is 2:
    problem = str(number_one) + " - " + str(number_two)
    solution = number_one - number_two
    user_solution = get_user_solution(problem, player)
    count = check_solution(user_solution, solution, count)
    return count
    elif index is 3:
    problem = str(number_one) + " * " + str(number_two)
    solution = number_one * number_two
    user_solution = get_user_solution(problem, player)
    count = check_solution(user_solution, solution, count)
    return count
    else:
    problem = str(number_one) + " // " + str(number_two)
    solution = number_one // number_two
    user_solution = get_user_solution(problem, player)
    count = check_solution(user_solution, solution, count)
    return count

    def display_result(total, correct):
    if total > 0:
    result = correct / total
    percentage = round((result * 100), 2)
    if total == 0:
    percentage = 0
    print("You answered", total, "questions with", correct, "correct.")
    print("Your score is ", percentage, "%. Thank you.", sep = "")

    winner = min(players,key=lambda a: a['points'])['name']
    print("The winner is...")
    time.sleep(0.5)
    print("{winner}!")

    def play_turn(player):
    if SIMU:
    print("It is now 's turn.nBe ready.")
    else:
    input("It is now 's turn.nPress enter when ready.")
    display_intro()
    display_menu()
    display_separator()

    option = get_user_input()
    total = 0
    correct = 0
    while option != 5:
    total = total + 1
    correct = menu_option(option, correct, player)
    option = get_user_input()

    print("Exit the quiz.")
    display_separator()
    display_result(total, correct)

    def play_round(players):
    print("-"*20 + "nIt is now round !n" + "-"*20)

    for player in players:
    play_turn(player)

    def play_game():
    if SIMU:
    players = [{'name': name, 'points': 0} for name in ('foo', 'bar', 'foobar')]
    nb_rounds = 1
    else:
    players = [{
    'name':input("Player: "),
    'points':0
    } for i in range(int(input("How many players? ")))]
    nb_rounds = int(input("How many rounds? "))
    for i in range(0, nb_rounds):
    play_round(players)

    play_game()


    Now, it it easier to improve these functions.



    Improve display_intro



    We could compute the length and the border string in a single place:



    def display_intro():
    title = "** A Simple Math Quiz **"
    border = "*" * len(title)
    print(border)
    print(title)
    print(border)


    Improve display_menu



    It is a good idea to have the menu in a proper data structure. It would be even better too iterate over it instead of using indices: it would be more concise, more efficient and easier to maintain (you do not have to remember to add/remove lines when you add/remove elements from the menu).



    def display_menu():
    menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
    for menu_item in menu_list:
    print(menu_item)


    Also, we could have the numbers generated automatically but we'll come back to this.



    Improve check_solution



    You have the same thing returned in both branches. You could have it in a single place:



    def check_solution(user_solution, solution, count):
    if user_solution == solution:
    count = count + 1
    print("Correct.")
    else:
    print("Incorrect.")
    return count


    Improving display_result



    You could use elif at the beginning because the 2 cases are mutually exclusive:



    if total > 0:
    result = correct / total
    percentage = round((result * 100), 2)
    elif total == 0:
    percentage = 0


    However, I'd rather have this written:



    if total == 0:
    percentage = 0
    else:
    result = correct / total
    percentage = round((result * 100), 2)


    Improve menu_option



    The main thing one can see is that it contains highly duplicated code.



    Let's extract out the common part to make things clearer:



    def menu_option(index, count, player):
    number_one = random.randrange(1, 21)
    number_two = random.randrange(1, 21)
    if index is 1:
    op_str = " + "
    res = number_one + number_two
    elif index is 2:
    op_str = " - "
    res = number_one - number_two
    elif index is 3:
    op_str = " * "
    res = number_one * number_two
    else:
    op_str = " // "
    res = number_one // number_two
    problem = str(number_one) + op_str + str(number_two)
    user_solution = get_user_solution(problem, player)
    count = check_solution(user_solution, res, count)
    return count


    You use is to compare values. You should actually use == at that stage. You'll find more online about the is operator.



    Also, instead of writing that code with multiple conditions, we could try to fit the relevant data in a proper data structure, for instance a dictionnary mapping indices to information about the operation to be performed. The operator module will be helpful here (you could also use lambda functions).



    You'd have something like:



    import operator
    def menu_option(index, count, player):
    number_one = random.randrange(1, 21)
    number_two = random.randrange(1, 21)
    operations = {
    1: (" + ", operator.add),
    2: (" - ", operator.sub),
    3: (" * ", operator.mul),
    4: (" // ", operator.floordiv),
    }
    op_str, op_func = operations[index]
    res = op_func(number_one, number_two)
    problem = str(number_one) + op_str + str(number_two)
    count = check_solution(get_user_solution(problem, player), res, count)
    return count


    Now, we can think about the real meaning of the keys of that dict: they correspond to the user inputs we display in display_menu.



    If we want to add or remove a supported mathematical operation, it is very hard to get it right. We should try to use a Single Source of Truth.



    Then we also realise that the same information is also used in get_user_input along with the input to exit. It would make sense to get rid of that magic number which appears also in play_turn.



    We could write something like:



    OPERATIONS = {
    1: (" + ", "Addition", operator.add),
    2: (" - ", "Substraction", operator.sub),
    3: (" * ", "Multiplication", operator.mul),
    4: (" // ", "Integer Division", operator.floordiv),
    }
    EXIT_KEY = 5

    def display_menu():
    for k, v in OPERATIONS.items():
    op_str, op_name, op_func = v
    print(str(k) + ". " + op_name)
    print(str(EXIT_KEY) + ". Exit")

    def get_user_input():
    user_input = int(input("Enter your choice: "))
    while True:
    if user_input in OPERATIONS or user_input == EXIT_KEY:
    return user_input
    print("Invalid menu option.")
    user_input = int(input("Please try again: "))


    Now, adding or removing an operation is much more trivial. Also, if we want to start to use letters instead of numbers for the menu options, it is quite easy to do as well.



    Details



    total = total + 1 can be written total += 1.



    Small functions and responsabilities



    Instead of having check_solution called with a count value and returning the value incremented or not, we could ensure the value just returns a boolean and the calling function is reponsible for using it.



    def menu_option(index, count, player):
    n1 = random.randrange(1, 21)
    n2 = random.randrange(1, 21)
    op_str, op_name, op_func = OPERATIONS[index]
    res = op_func(n1, n2)
    problem = str(n1) + op_str + str(n2)
    if check_solution(get_user_solution(problem, player), res):
    count += 1
    return count

    def check_solution(user_solution, solution):
    correct = user_solution == solution
    print("Correct." if correct else "Incorrect.")
    return correct


    Now, the same logic actually applies to menu_option:



    def menu_option(index, player):
    n1 = random.randrange(1, 21)
    n2 = random.randrange(1, 21)
    op_str, op_name, op_func = OPERATIONS[index]
    res = op_func(n1, n2)
    problem = str(n1) + op_str + str(n2)
    return check_solution(get_user_solution(problem, player), res)

    def check_solution(user_solution, solution):
    correct = user_solution == solution
    print("Correct." if correct else "Incorrect.")
    return correct

    ...

    while option != EXIT_KEY:
    total += 1
    if menu_option(option, player):
    correct += 1
    option = get_user_input()


    At that stage, I start to feel that the function names could be improved.






    share|improve this answer











    $endgroup$


















      4












      $begingroup$

      Code organisation



      Having many functions defined in the middle of the core loop make things very hard to understand. It would be much easier and much more conventional to define all functions before the loop. One of the issues you might encounter is that functions will rely on variables they know nothing about such as players. A simple solution could be to add these as parameters.



      Also, main is a very confusing name for a function which is not the first thing launched. I guess play_turn would be a better name.



      At this stage, we'd have something like:



      Disclaimer: I've changed various details (removed fstrings because I don't have Python 3.6 available, etc)



      import random, time

      SIMU = True # hack for testing purposes

      def display_intro():
      title = "** A Simple Math Quiz **"
      print("*" * len(title))
      print(title)
      print("*" * len(title))

      def display_menu():
      menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
      print(menu_list[0])
      print(menu_list[1])
      print(menu_list[2])
      print(menu_list[3])
      print(menu_list[4])



      def display_separator():
      print("-" * 24)

      def get_user_input():
      user_input = int(input("Enter your choice: "))
      while user_input > 5 or user_input <= 0:
      print("Invalid menu option.")
      user_input = int(input("Please try again: "))
      else:
      return user_input

      def check_solution(user_solution, solution, count):
      if user_solution == solution:
      count = count + 1
      print("Correct.")
      return count
      else:
      print("Incorrect.")
      return count

      def get_user_solution(problem, player):
      if not SIMU:
      time.sleep(random.randint(1,8))
      then = time.time()
      print(problem, end="")
      result = int(input(" = "))
      reaction_time = time.time()-then
      print(" ms")
      player["points"] += reaction_time
      return result

      def menu_option(index, count, player):
      number_one = random.randrange(1, 21)
      number_two = random.randrange(1, 21)
      if index is 1:
      problem = str(number_one) + " + " + str(number_two)
      solution = number_one + number_two
      user_solution = get_user_solution(problem, player)
      count = check_solution(user_solution, solution, count)
      return count
      elif index is 2:
      problem = str(number_one) + " - " + str(number_two)
      solution = number_one - number_two
      user_solution = get_user_solution(problem, player)
      count = check_solution(user_solution, solution, count)
      return count
      elif index is 3:
      problem = str(number_one) + " * " + str(number_two)
      solution = number_one * number_two
      user_solution = get_user_solution(problem, player)
      count = check_solution(user_solution, solution, count)
      return count
      else:
      problem = str(number_one) + " // " + str(number_two)
      solution = number_one // number_two
      user_solution = get_user_solution(problem, player)
      count = check_solution(user_solution, solution, count)
      return count

      def display_result(total, correct):
      if total > 0:
      result = correct / total
      percentage = round((result * 100), 2)
      if total == 0:
      percentage = 0
      print("You answered", total, "questions with", correct, "correct.")
      print("Your score is ", percentage, "%. Thank you.", sep = "")

      winner = min(players,key=lambda a: a['points'])['name']
      print("The winner is...")
      time.sleep(0.5)
      print("{winner}!")

      def play_turn(player):
      if SIMU:
      print("It is now 's turn.nBe ready.")
      else:
      input("It is now 's turn.nPress enter when ready.")
      display_intro()
      display_menu()
      display_separator()

      option = get_user_input()
      total = 0
      correct = 0
      while option != 5:
      total = total + 1
      correct = menu_option(option, correct, player)
      option = get_user_input()

      print("Exit the quiz.")
      display_separator()
      display_result(total, correct)

      def play_round(players):
      print("-"*20 + "nIt is now round !n" + "-"*20)

      for player in players:
      play_turn(player)

      def play_game():
      if SIMU:
      players = [{'name': name, 'points': 0} for name in ('foo', 'bar', 'foobar')]
      nb_rounds = 1
      else:
      players = [{
      'name':input("Player: "),
      'points':0
      } for i in range(int(input("How many players? ")))]
      nb_rounds = int(input("How many rounds? "))
      for i in range(0, nb_rounds):
      play_round(players)

      play_game()


      Now, it it easier to improve these functions.



      Improve display_intro



      We could compute the length and the border string in a single place:



      def display_intro():
      title = "** A Simple Math Quiz **"
      border = "*" * len(title)
      print(border)
      print(title)
      print(border)


      Improve display_menu



      It is a good idea to have the menu in a proper data structure. It would be even better too iterate over it instead of using indices: it would be more concise, more efficient and easier to maintain (you do not have to remember to add/remove lines when you add/remove elements from the menu).



      def display_menu():
      menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
      for menu_item in menu_list:
      print(menu_item)


      Also, we could have the numbers generated automatically but we'll come back to this.



      Improve check_solution



      You have the same thing returned in both branches. You could have it in a single place:



      def check_solution(user_solution, solution, count):
      if user_solution == solution:
      count = count + 1
      print("Correct.")
      else:
      print("Incorrect.")
      return count


      Improving display_result



      You could use elif at the beginning because the 2 cases are mutually exclusive:



      if total > 0:
      result = correct / total
      percentage = round((result * 100), 2)
      elif total == 0:
      percentage = 0


      However, I'd rather have this written:



      if total == 0:
      percentage = 0
      else:
      result = correct / total
      percentage = round((result * 100), 2)


      Improve menu_option



      The main thing one can see is that it contains highly duplicated code.



      Let's extract out the common part to make things clearer:



      def menu_option(index, count, player):
      number_one = random.randrange(1, 21)
      number_two = random.randrange(1, 21)
      if index is 1:
      op_str = " + "
      res = number_one + number_two
      elif index is 2:
      op_str = " - "
      res = number_one - number_two
      elif index is 3:
      op_str = " * "
      res = number_one * number_two
      else:
      op_str = " // "
      res = number_one // number_two
      problem = str(number_one) + op_str + str(number_two)
      user_solution = get_user_solution(problem, player)
      count = check_solution(user_solution, res, count)
      return count


      You use is to compare values. You should actually use == at that stage. You'll find more online about the is operator.



      Also, instead of writing that code with multiple conditions, we could try to fit the relevant data in a proper data structure, for instance a dictionnary mapping indices to information about the operation to be performed. The operator module will be helpful here (you could also use lambda functions).



      You'd have something like:



      import operator
      def menu_option(index, count, player):
      number_one = random.randrange(1, 21)
      number_two = random.randrange(1, 21)
      operations = {
      1: (" + ", operator.add),
      2: (" - ", operator.sub),
      3: (" * ", operator.mul),
      4: (" // ", operator.floordiv),
      }
      op_str, op_func = operations[index]
      res = op_func(number_one, number_two)
      problem = str(number_one) + op_str + str(number_two)
      count = check_solution(get_user_solution(problem, player), res, count)
      return count


      Now, we can think about the real meaning of the keys of that dict: they correspond to the user inputs we display in display_menu.



      If we want to add or remove a supported mathematical operation, it is very hard to get it right. We should try to use a Single Source of Truth.



      Then we also realise that the same information is also used in get_user_input along with the input to exit. It would make sense to get rid of that magic number which appears also in play_turn.



      We could write something like:



      OPERATIONS = {
      1: (" + ", "Addition", operator.add),
      2: (" - ", "Substraction", operator.sub),
      3: (" * ", "Multiplication", operator.mul),
      4: (" // ", "Integer Division", operator.floordiv),
      }
      EXIT_KEY = 5

      def display_menu():
      for k, v in OPERATIONS.items():
      op_str, op_name, op_func = v
      print(str(k) + ". " + op_name)
      print(str(EXIT_KEY) + ". Exit")

      def get_user_input():
      user_input = int(input("Enter your choice: "))
      while True:
      if user_input in OPERATIONS or user_input == EXIT_KEY:
      return user_input
      print("Invalid menu option.")
      user_input = int(input("Please try again: "))


      Now, adding or removing an operation is much more trivial. Also, if we want to start to use letters instead of numbers for the menu options, it is quite easy to do as well.



      Details



      total = total + 1 can be written total += 1.



      Small functions and responsabilities



      Instead of having check_solution called with a count value and returning the value incremented or not, we could ensure the value just returns a boolean and the calling function is reponsible for using it.



      def menu_option(index, count, player):
      n1 = random.randrange(1, 21)
      n2 = random.randrange(1, 21)
      op_str, op_name, op_func = OPERATIONS[index]
      res = op_func(n1, n2)
      problem = str(n1) + op_str + str(n2)
      if check_solution(get_user_solution(problem, player), res):
      count += 1
      return count

      def check_solution(user_solution, solution):
      correct = user_solution == solution
      print("Correct." if correct else "Incorrect.")
      return correct


      Now, the same logic actually applies to menu_option:



      def menu_option(index, player):
      n1 = random.randrange(1, 21)
      n2 = random.randrange(1, 21)
      op_str, op_name, op_func = OPERATIONS[index]
      res = op_func(n1, n2)
      problem = str(n1) + op_str + str(n2)
      return check_solution(get_user_solution(problem, player), res)

      def check_solution(user_solution, solution):
      correct = user_solution == solution
      print("Correct." if correct else "Incorrect.")
      return correct

      ...

      while option != EXIT_KEY:
      total += 1
      if menu_option(option, player):
      correct += 1
      option = get_user_input()


      At that stage, I start to feel that the function names could be improved.






      share|improve this answer











      $endgroup$
















        4












        4








        4





        $begingroup$

        Code organisation



        Having many functions defined in the middle of the core loop make things very hard to understand. It would be much easier and much more conventional to define all functions before the loop. One of the issues you might encounter is that functions will rely on variables they know nothing about such as players. A simple solution could be to add these as parameters.



        Also, main is a very confusing name for a function which is not the first thing launched. I guess play_turn would be a better name.



        At this stage, we'd have something like:



        Disclaimer: I've changed various details (removed fstrings because I don't have Python 3.6 available, etc)



        import random, time

        SIMU = True # hack for testing purposes

        def display_intro():
        title = "** A Simple Math Quiz **"
        print("*" * len(title))
        print(title)
        print("*" * len(title))

        def display_menu():
        menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
        print(menu_list[0])
        print(menu_list[1])
        print(menu_list[2])
        print(menu_list[3])
        print(menu_list[4])



        def display_separator():
        print("-" * 24)

        def get_user_input():
        user_input = int(input("Enter your choice: "))
        while user_input > 5 or user_input <= 0:
        print("Invalid menu option.")
        user_input = int(input("Please try again: "))
        else:
        return user_input

        def check_solution(user_solution, solution, count):
        if user_solution == solution:
        count = count + 1
        print("Correct.")
        return count
        else:
        print("Incorrect.")
        return count

        def get_user_solution(problem, player):
        if not SIMU:
        time.sleep(random.randint(1,8))
        then = time.time()
        print(problem, end="")
        result = int(input(" = "))
        reaction_time = time.time()-then
        print(" ms")
        player["points"] += reaction_time
        return result

        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        if index is 1:
        problem = str(number_one) + " + " + str(number_two)
        solution = number_one + number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        elif index is 2:
        problem = str(number_one) + " - " + str(number_two)
        solution = number_one - number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        elif index is 3:
        problem = str(number_one) + " * " + str(number_two)
        solution = number_one * number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        else:
        problem = str(number_one) + " // " + str(number_two)
        solution = number_one // number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count

        def display_result(total, correct):
        if total > 0:
        result = correct / total
        percentage = round((result * 100), 2)
        if total == 0:
        percentage = 0
        print("You answered", total, "questions with", correct, "correct.")
        print("Your score is ", percentage, "%. Thank you.", sep = "")

        winner = min(players,key=lambda a: a['points'])['name']
        print("The winner is...")
        time.sleep(0.5)
        print("{winner}!")

        def play_turn(player):
        if SIMU:
        print("It is now 's turn.nBe ready.")
        else:
        input("It is now 's turn.nPress enter when ready.")
        display_intro()
        display_menu()
        display_separator()

        option = get_user_input()
        total = 0
        correct = 0
        while option != 5:
        total = total + 1
        correct = menu_option(option, correct, player)
        option = get_user_input()

        print("Exit the quiz.")
        display_separator()
        display_result(total, correct)

        def play_round(players):
        print("-"*20 + "nIt is now round !n" + "-"*20)

        for player in players:
        play_turn(player)

        def play_game():
        if SIMU:
        players = [{'name': name, 'points': 0} for name in ('foo', 'bar', 'foobar')]
        nb_rounds = 1
        else:
        players = [{
        'name':input("Player: "),
        'points':0
        } for i in range(int(input("How many players? ")))]
        nb_rounds = int(input("How many rounds? "))
        for i in range(0, nb_rounds):
        play_round(players)

        play_game()


        Now, it it easier to improve these functions.



        Improve display_intro



        We could compute the length and the border string in a single place:



        def display_intro():
        title = "** A Simple Math Quiz **"
        border = "*" * len(title)
        print(border)
        print(title)
        print(border)


        Improve display_menu



        It is a good idea to have the menu in a proper data structure. It would be even better too iterate over it instead of using indices: it would be more concise, more efficient and easier to maintain (you do not have to remember to add/remove lines when you add/remove elements from the menu).



        def display_menu():
        menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
        for menu_item in menu_list:
        print(menu_item)


        Also, we could have the numbers generated automatically but we'll come back to this.



        Improve check_solution



        You have the same thing returned in both branches. You could have it in a single place:



        def check_solution(user_solution, solution, count):
        if user_solution == solution:
        count = count + 1
        print("Correct.")
        else:
        print("Incorrect.")
        return count


        Improving display_result



        You could use elif at the beginning because the 2 cases are mutually exclusive:



        if total > 0:
        result = correct / total
        percentage = round((result * 100), 2)
        elif total == 0:
        percentage = 0


        However, I'd rather have this written:



        if total == 0:
        percentage = 0
        else:
        result = correct / total
        percentage = round((result * 100), 2)


        Improve menu_option



        The main thing one can see is that it contains highly duplicated code.



        Let's extract out the common part to make things clearer:



        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        if index is 1:
        op_str = " + "
        res = number_one + number_two
        elif index is 2:
        op_str = " - "
        res = number_one - number_two
        elif index is 3:
        op_str = " * "
        res = number_one * number_two
        else:
        op_str = " // "
        res = number_one // number_two
        problem = str(number_one) + op_str + str(number_two)
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, res, count)
        return count


        You use is to compare values. You should actually use == at that stage. You'll find more online about the is operator.



        Also, instead of writing that code with multiple conditions, we could try to fit the relevant data in a proper data structure, for instance a dictionnary mapping indices to information about the operation to be performed. The operator module will be helpful here (you could also use lambda functions).



        You'd have something like:



        import operator
        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        operations = {
        1: (" + ", operator.add),
        2: (" - ", operator.sub),
        3: (" * ", operator.mul),
        4: (" // ", operator.floordiv),
        }
        op_str, op_func = operations[index]
        res = op_func(number_one, number_two)
        problem = str(number_one) + op_str + str(number_two)
        count = check_solution(get_user_solution(problem, player), res, count)
        return count


        Now, we can think about the real meaning of the keys of that dict: they correspond to the user inputs we display in display_menu.



        If we want to add or remove a supported mathematical operation, it is very hard to get it right. We should try to use a Single Source of Truth.



        Then we also realise that the same information is also used in get_user_input along with the input to exit. It would make sense to get rid of that magic number which appears also in play_turn.



        We could write something like:



        OPERATIONS = {
        1: (" + ", "Addition", operator.add),
        2: (" - ", "Substraction", operator.sub),
        3: (" * ", "Multiplication", operator.mul),
        4: (" // ", "Integer Division", operator.floordiv),
        }
        EXIT_KEY = 5

        def display_menu():
        for k, v in OPERATIONS.items():
        op_str, op_name, op_func = v
        print(str(k) + ". " + op_name)
        print(str(EXIT_KEY) + ". Exit")

        def get_user_input():
        user_input = int(input("Enter your choice: "))
        while True:
        if user_input in OPERATIONS or user_input == EXIT_KEY:
        return user_input
        print("Invalid menu option.")
        user_input = int(input("Please try again: "))


        Now, adding or removing an operation is much more trivial. Also, if we want to start to use letters instead of numbers for the menu options, it is quite easy to do as well.



        Details



        total = total + 1 can be written total += 1.



        Small functions and responsabilities



        Instead of having check_solution called with a count value and returning the value incremented or not, we could ensure the value just returns a boolean and the calling function is reponsible for using it.



        def menu_option(index, count, player):
        n1 = random.randrange(1, 21)
        n2 = random.randrange(1, 21)
        op_str, op_name, op_func = OPERATIONS[index]
        res = op_func(n1, n2)
        problem = str(n1) + op_str + str(n2)
        if check_solution(get_user_solution(problem, player), res):
        count += 1
        return count

        def check_solution(user_solution, solution):
        correct = user_solution == solution
        print("Correct." if correct else "Incorrect.")
        return correct


        Now, the same logic actually applies to menu_option:



        def menu_option(index, player):
        n1 = random.randrange(1, 21)
        n2 = random.randrange(1, 21)
        op_str, op_name, op_func = OPERATIONS[index]
        res = op_func(n1, n2)
        problem = str(n1) + op_str + str(n2)
        return check_solution(get_user_solution(problem, player), res)

        def check_solution(user_solution, solution):
        correct = user_solution == solution
        print("Correct." if correct else "Incorrect.")
        return correct

        ...

        while option != EXIT_KEY:
        total += 1
        if menu_option(option, player):
        correct += 1
        option = get_user_input()


        At that stage, I start to feel that the function names could be improved.






        share|improve this answer











        $endgroup$



        Code organisation



        Having many functions defined in the middle of the core loop make things very hard to understand. It would be much easier and much more conventional to define all functions before the loop. One of the issues you might encounter is that functions will rely on variables they know nothing about such as players. A simple solution could be to add these as parameters.



        Also, main is a very confusing name for a function which is not the first thing launched. I guess play_turn would be a better name.



        At this stage, we'd have something like:



        Disclaimer: I've changed various details (removed fstrings because I don't have Python 3.6 available, etc)



        import random, time

        SIMU = True # hack for testing purposes

        def display_intro():
        title = "** A Simple Math Quiz **"
        print("*" * len(title))
        print(title)
        print("*" * len(title))

        def display_menu():
        menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
        print(menu_list[0])
        print(menu_list[1])
        print(menu_list[2])
        print(menu_list[3])
        print(menu_list[4])



        def display_separator():
        print("-" * 24)

        def get_user_input():
        user_input = int(input("Enter your choice: "))
        while user_input > 5 or user_input <= 0:
        print("Invalid menu option.")
        user_input = int(input("Please try again: "))
        else:
        return user_input

        def check_solution(user_solution, solution, count):
        if user_solution == solution:
        count = count + 1
        print("Correct.")
        return count
        else:
        print("Incorrect.")
        return count

        def get_user_solution(problem, player):
        if not SIMU:
        time.sleep(random.randint(1,8))
        then = time.time()
        print(problem, end="")
        result = int(input(" = "))
        reaction_time = time.time()-then
        print(" ms")
        player["points"] += reaction_time
        return result

        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        if index is 1:
        problem = str(number_one) + " + " + str(number_two)
        solution = number_one + number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        elif index is 2:
        problem = str(number_one) + " - " + str(number_two)
        solution = number_one - number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        elif index is 3:
        problem = str(number_one) + " * " + str(number_two)
        solution = number_one * number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count
        else:
        problem = str(number_one) + " // " + str(number_two)
        solution = number_one // number_two
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, solution, count)
        return count

        def display_result(total, correct):
        if total > 0:
        result = correct / total
        percentage = round((result * 100), 2)
        if total == 0:
        percentage = 0
        print("You answered", total, "questions with", correct, "correct.")
        print("Your score is ", percentage, "%. Thank you.", sep = "")

        winner = min(players,key=lambda a: a['points'])['name']
        print("The winner is...")
        time.sleep(0.5)
        print("{winner}!")

        def play_turn(player):
        if SIMU:
        print("It is now 's turn.nBe ready.")
        else:
        input("It is now 's turn.nPress enter when ready.")
        display_intro()
        display_menu()
        display_separator()

        option = get_user_input()
        total = 0
        correct = 0
        while option != 5:
        total = total + 1
        correct = menu_option(option, correct, player)
        option = get_user_input()

        print("Exit the quiz.")
        display_separator()
        display_result(total, correct)

        def play_round(players):
        print("-"*20 + "nIt is now round !n" + "-"*20)

        for player in players:
        play_turn(player)

        def play_game():
        if SIMU:
        players = [{'name': name, 'points': 0} for name in ('foo', 'bar', 'foobar')]
        nb_rounds = 1
        else:
        players = [{
        'name':input("Player: "),
        'points':0
        } for i in range(int(input("How many players? ")))]
        nb_rounds = int(input("How many rounds? "))
        for i in range(0, nb_rounds):
        play_round(players)

        play_game()


        Now, it it easier to improve these functions.



        Improve display_intro



        We could compute the length and the border string in a single place:



        def display_intro():
        title = "** A Simple Math Quiz **"
        border = "*" * len(title)
        print(border)
        print(title)
        print(border)


        Improve display_menu



        It is a good idea to have the menu in a proper data structure. It would be even better too iterate over it instead of using indices: it would be more concise, more efficient and easier to maintain (you do not have to remember to add/remove lines when you add/remove elements from the menu).



        def display_menu():
        menu_list = ["1. Addition", "2. Subtraction", "3. Multiplication", "4. Integer Division", "5. Exit"]
        for menu_item in menu_list:
        print(menu_item)


        Also, we could have the numbers generated automatically but we'll come back to this.



        Improve check_solution



        You have the same thing returned in both branches. You could have it in a single place:



        def check_solution(user_solution, solution, count):
        if user_solution == solution:
        count = count + 1
        print("Correct.")
        else:
        print("Incorrect.")
        return count


        Improving display_result



        You could use elif at the beginning because the 2 cases are mutually exclusive:



        if total > 0:
        result = correct / total
        percentage = round((result * 100), 2)
        elif total == 0:
        percentage = 0


        However, I'd rather have this written:



        if total == 0:
        percentage = 0
        else:
        result = correct / total
        percentage = round((result * 100), 2)


        Improve menu_option



        The main thing one can see is that it contains highly duplicated code.



        Let's extract out the common part to make things clearer:



        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        if index is 1:
        op_str = " + "
        res = number_one + number_two
        elif index is 2:
        op_str = " - "
        res = number_one - number_two
        elif index is 3:
        op_str = " * "
        res = number_one * number_two
        else:
        op_str = " // "
        res = number_one // number_two
        problem = str(number_one) + op_str + str(number_two)
        user_solution = get_user_solution(problem, player)
        count = check_solution(user_solution, res, count)
        return count


        You use is to compare values. You should actually use == at that stage. You'll find more online about the is operator.



        Also, instead of writing that code with multiple conditions, we could try to fit the relevant data in a proper data structure, for instance a dictionnary mapping indices to information about the operation to be performed. The operator module will be helpful here (you could also use lambda functions).



        You'd have something like:



        import operator
        def menu_option(index, count, player):
        number_one = random.randrange(1, 21)
        number_two = random.randrange(1, 21)
        operations = {
        1: (" + ", operator.add),
        2: (" - ", operator.sub),
        3: (" * ", operator.mul),
        4: (" // ", operator.floordiv),
        }
        op_str, op_func = operations[index]
        res = op_func(number_one, number_two)
        problem = str(number_one) + op_str + str(number_two)
        count = check_solution(get_user_solution(problem, player), res, count)
        return count


        Now, we can think about the real meaning of the keys of that dict: they correspond to the user inputs we display in display_menu.



        If we want to add or remove a supported mathematical operation, it is very hard to get it right. We should try to use a Single Source of Truth.



        Then we also realise that the same information is also used in get_user_input along with the input to exit. It would make sense to get rid of that magic number which appears also in play_turn.



        We could write something like:



        OPERATIONS = {
        1: (" + ", "Addition", operator.add),
        2: (" - ", "Substraction", operator.sub),
        3: (" * ", "Multiplication", operator.mul),
        4: (" // ", "Integer Division", operator.floordiv),
        }
        EXIT_KEY = 5

        def display_menu():
        for k, v in OPERATIONS.items():
        op_str, op_name, op_func = v
        print(str(k) + ". " + op_name)
        print(str(EXIT_KEY) + ". Exit")

        def get_user_input():
        user_input = int(input("Enter your choice: "))
        while True:
        if user_input in OPERATIONS or user_input == EXIT_KEY:
        return user_input
        print("Invalid menu option.")
        user_input = int(input("Please try again: "))


        Now, adding or removing an operation is much more trivial. Also, if we want to start to use letters instead of numbers for the menu options, it is quite easy to do as well.



        Details



        total = total + 1 can be written total += 1.



        Small functions and responsabilities



        Instead of having check_solution called with a count value and returning the value incremented or not, we could ensure the value just returns a boolean and the calling function is reponsible for using it.



        def menu_option(index, count, player):
        n1 = random.randrange(1, 21)
        n2 = random.randrange(1, 21)
        op_str, op_name, op_func = OPERATIONS[index]
        res = op_func(n1, n2)
        problem = str(n1) + op_str + str(n2)
        if check_solution(get_user_solution(problem, player), res):
        count += 1
        return count

        def check_solution(user_solution, solution):
        correct = user_solution == solution
        print("Correct." if correct else "Incorrect.")
        return correct


        Now, the same logic actually applies to menu_option:



        def menu_option(index, player):
        n1 = random.randrange(1, 21)
        n2 = random.randrange(1, 21)
        op_str, op_name, op_func = OPERATIONS[index]
        res = op_func(n1, n2)
        problem = str(n1) + op_str + str(n2)
        return check_solution(get_user_solution(problem, player), res)

        def check_solution(user_solution, solution):
        correct = user_solution == solution
        print("Correct." if correct else "Incorrect.")
        return correct

        ...

        while option != EXIT_KEY:
        total += 1
        if menu_option(option, player):
        correct += 1
        option = get_user_input()


        At that stage, I start to feel that the function names could be improved.







        share|improve this answer














        share|improve this answer



        share|improve this answer








        edited Feb 7 at 22:39

























        answered Feb 5 at 0:59









        JosayJosay

        25.8k14087




        25.8k14087






























            draft saved

            draft discarded




















































            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.




            draft saved


            draft discarded














            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f212884%2fexpired%23new-answer', 'question_page');
            }
            );

            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







            Popular posts from this blog

            404 Error Contact Form 7 ajax form submitting

            How to know if a Active Directory user can login interactively

            TypeError: fit_transform() missing 1 required positional argument: 'X'