8
\$\begingroup\$

Im making a Python program. It is basically a questionnaire and I was just wondering if there is a shorter way of doing this as at the moment I'm writing out all of the questions in separate functions. I was wondering if there is an alternative way of doing this in almost a single function where the function just repeats but the question and right answers change every time. If I cannot do this is there an alternate way to do this without having to do separate functions for each question? is there a faster/more efficient way of completing this?

All advice would be much appreciated and I have pasted the first 4 questions and the rest of the code below. I will be having many more questions but i just did 4 so that it wouldn't take up too much space :)

import time
questionNumber = 0
right = 0
wrong = 0
name = str(input("Please Enter your name"))
print()
def questions():
    def questionOne():
        global right, wrong, questionNumber
        print("What is the population of New Zealand?")
        print("Is it A:6.7m B:3.2m C:5.1m or D:4.5m")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '4.5' or ans == '4':
            print("You got it right!")
            right = right + 1
        else:
            print("You got it wrong!")
            wrong = wrong+1
        questionNumber = questionNumber + 1
    questionOne()
    time.sleep(2)
    print()
    print("So far",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions")
    print()
    time.sleep(4)
    def questionTwo():
        global right, wrong, questionNumber
        print("What year did the first european set foot on New Zealand (Abel Tasman)")
        print("Is it A:1830 B:1543 C:1642 or D:1765")
        ans = str(input())
        if ans == "C" or ans == "c" or ans == '1642'or ans == '3':
            print("You got it right!")
            right = right + 1
        else:
            print("You got it wrong!")
            wrong = wrong+1
        questionNumber = questionNumber + 1   
    questionTwo()
    time.sleep(2)
    print()
    print("So far ",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions")
    print()
    time.sleep(4)
    def questionThree():
        global right, wrong, questionNumber
        print("How many Kiwi are there left in New Zealand (Approx)")
        print("Is it A:2000 B:600 C:70,000 or D:100000")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '100000'or ans == '4':
            print("You got it right!")
            right = right + 1
        else:
            print("You got it wrong!")
            wrong = wrong+1
        questionNumber = questionNumber + 1   
    questionThree()
    time.sleep(2)
    print()
    print("So far ",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions")
    print()
    time.sleep(4)
    def questionFour():
        global right, wrong, questionNumber
        print("How many new babys where born in New Zealand in 2015")
        print("Is it A:61,000 B:208,000 C:98,000 or D:18,000")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '100000'or ans == '4':S
            print("You got it right!")
            right = right + 1
        else:
            print("You got it wrong!")
            wrong = wrong+1
        questionNumber = questionNumber + 1   
    questionFour()
questions()
\$\endgroup\$
  • 4
    \$\begingroup\$ Welcome to Code Review! As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ – 301_Moved_Permanently Jul 5 '16 at 8:39
  • \$\begingroup\$ Does your code run as it is ? I'm a bit skeptical because of this if ans == "D" or ans == "d" or ans == '100000'or ans == '4':S. More, in the second last line of your program, you do questionFour() which would be unresolved. Could you please edit your question ? Or do you need any help with it ? \$\endgroup\$ – Grajdeanu Alex. Jul 5 '16 at 9:08
  • \$\begingroup\$ Function arguments can help you. Passing them into a function modifies its behaviour. def foo(x, y): ... where x and y are the arguments. Question text and choices can be arguments \$\endgroup\$ – Caridorc Jul 5 '16 at 11:03
10
\$\begingroup\$

I'll base my comments on Dex'ter's excellent answer as he covered most of the points I wanted to hilight but there are a few details that I'd try to do in a slightly different way. Among these, as pointed out by Caridorc, I'd rather avoid parallel lists and use a more direct approach.

question_list = [  # tuple of the form (question, answer list, correct answer list)
    ('What is the population of New Zealand ?',
    ['A:1830', 'B:1543', 'C:1642', 'D:1765'],
    ['D', 'd', '4.5', '4']),

    ('What year did the first european set foot on New Zealand (Abel Tasman) ?',
    ['A:1830', 'B:1543', 'C:1642', 'D:1765'],
    ['C', 'c', '1642', '3']),

    ('How many Kiwi are there left in New Zealand (Approx) ?',
    ['A:2000', 'B:600',  'C:70,000', 'D:100000'],
    ['D', 'd', '100000', '4']),

    ('How many new babys where born in New Zealand in 2015 ?',
    ['A:61,000', 'B:208,000', 'C:98,000', 'D:18,000'],
    ['D', 'd', '100000', '4']),
]

def questions():
    wrong = 0
    right = 0

    for each_question, each_answer, each_correct_answer in question_list:
        print(each_question + '\n' + ' '.join(each_answer))
        get_answer = raw_input()

        if get_answer in each_correct_answer:
            print('Your answer is correct!\n')
            right += 1
        else:
            print('That is not the answer I had in mind!\n')
            wrong += 1
        print('So far, you answered correctly to {0} questions and incorrectly to {1}. Good luck!'.format(right, wrong))


if __name__ == '__main__':
    questions()

Now things could be improved by having a single source of information. At the moment, some information (like the correct answer) is repeated in multiple places. It is easy to get it wrong and you did get it wrong for the last correct answer : ['D', 'd', '100000', '4']. Also, it could be a good idea to separate the concerns between what is actual data and what is used for user input/ouput interface.

For instance, from the following code:

from random import shuffle

question_list = [  # tuple of the form (question, dict of answers)
    ('What is the population of New Zealand ?',
    {'6.7': False, '3.2': False, '5.1': False, '4.5': True}),

    ('What year did the first european set foot on New Zealand Abel Tasman ?',
    {'1830': False, '1543': False, '1765': False, '1642': True}),

    ('How many Kiwi are there left in New Zealand Approx ?',
    {'2000': False, '600': False,  '70,000': False, '100000': False, '100000': True}),

    ('How many new babys where born in New Zealand in 2015 ?',
    {'61,000': False, '208,000': False, '98,000': False, '18,000': True}),
]


def get_input_in_list(lst):
    while True:
        print("Please enter value from : " + " ".join(lst))
        user_input = raw_input()
        if user_input in lst:
            return user_input


def questions():
    wrong = 0
    right = 0

    for each_question, answer_dict in question_list:
        answers = list(answer_dict)
        shuffle(answers)
        print(each_question)
        user_answer = get_input_in_list(answers) 

        if answer_dict[user_answer]:
            print('Your answer is correct!\n')
            right += 1
        else:
            print('That is not the answer I had in mind!\n')
            wrong += 1
        print('So far, you answered correctly to {0} questions and incorrectly to {1}. Good luck!'.format(right, wrong))


if __name__ == '__main__':
    questions()

You can easily add feature to select answers by letters/index by updating the get_input_in_list without messing with the data corresponding to your questions/answers.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Nice. Thanks for the reference of SSOT. I learn new things everyday \$\endgroup\$ – Grajdeanu Alex. Jul 5 '16 at 12:18
  • 2
    \$\begingroup\$ You're most welcome! Learning (and teaching what I've learnt) is the very reason why I am here :) \$\endgroup\$ – SylvainD Jul 5 '16 at 12:26
  • 1
    \$\begingroup\$ Josay and Dex'ter, all good hints, I have one more, try to limit the line length. Besides all the good reasons in PEP8, it makes the code boxes from CodeReview and StackOverflow readable without having to use the horizontal scroll bars. \$\endgroup\$ – Jan Kuiken Jul 5 '16 at 18:01
  • \$\begingroup\$ @CanadianLuke Nice catch! I've fixed this. \$\endgroup\$ – SylvainD Jul 6 '16 at 8:45
10
\$\begingroup\$

Some PEP8(which is a Style Guide for Python Code) comments regarding your coding style and formatting:

1. Spacing

Between methods, you should have two blank lines.

def questionOne():
    ....

def questionTwo():
    ....

2. Naming conventions

In PEP8 it's recommended to:

Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.

That said, questionNumber would become question_number

I've seen many people using also camelCase for variables, so that's rather a matter of preference.

The same rule applies for your methods:

questionOne() -> question_one()

Put a space after each comma (,). It really helps when going through your code.

So this:

print("\nSo far ",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions\n")

Might be rewritten as:

print("\nSo far ", name, "You have got", right, "Answers right,", wrong, "Answers wrong and you have completed",
      questionNumber, "Questions\n")

3. Other comments

Instead of right = right + 1 you can use an augmented assignment: right += 1.

More, instead of doing this:

print()
print("So far",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions")
print()

You can do:

print("\nSo far",name,"You have got",right,"Answers right,",wrong,"Answers wrong and you have completed",questionNumber,"Questions\n")

I don't like how you formatted the above print() but I'll come back to it later on.

Let's see what we've got so far using the modifications I mentioned so far:

import time

wrong = 0
right = 0
question_number = 0
name = str(input("Please Enter your name: "))


def questions():
    def question_one():
        global right, wrong, question_number
        print("What is the population of New Zealand?")
        print("Is it A:6.7m B:3.2m C:5.1m or D:4.5m")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '4.5' or ans == '4':
            print("You got it right!")
            right += 1
        else:
            print("You got it wrong!")
            wrong += 1
        question_number += 1

    question_one()
    time.sleep(2)
    print("\nSo far", name, "You have got", right, "Answers right,", wrong, "Answers wrong and you have completed",
          question_number, "Questions\n")
    time.sleep(4)

    def question_two():
        global right, wrong, question_number
        print("What year did the first european set foot on New Zealand (Abel Tasman)")
        print("Is it A:1830 B:1543 C:1642 or D:1765")
        ans = str(input())
        if ans == "C" or ans == "c" or ans == '1642' or ans == '3':
            print("You got it right!")
            right += 1
        else:
            print("You got it wrong!")
            wrong += 1
        question_number += 1

    question_two()
    time.sleep(2)
    print("\nSo far ", name, "You have got", right, "Answers right,", wrong, "Answers wrong and you have completed",
          question_number, "Questions\n")
    time.sleep(4)

    def question_three():
        global right, wrong, question_number
        print("How many Kiwi are there left in New Zealand (Approx)")
        print("Is it A:2000 B:600 C:70,000 or D:100000")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '100000' or ans == '4':
            print("You got it right!")
            right += 1
        else:
            print("You got it wrong!")
            wrong += 1
        question_number += 1

    question_three()
    time.sleep(2)
    print("\nSo far ", name, "You have got", right, "Answers right,", wrong, "Answers wrong and you have completed",
          question_number, "Questions\n")
    time.sleep(4)

    def question_four():
        global right, wrong, question_number
        print("How many new babys where born in New Zealand in 2015")
        print("Is it A:61,000 B:208,000 C:98,000 or D:18,000")
        ans = str(input())
        if ans == "D" or ans == "d" or ans == '100000' or ans == '4':
            print("You got it right!")
            right += 1
        else:
            print("You got it wrong!")
            wrong += 1
        question_number += 1

    question_four()

questions()

Digging deeper:

Globals

Try to avoid using globals as much as possible. Global variables should be avoided because they inhibit code reuse. The reason they are bad is that they allow functions to have hidden (as in "non-obvious" and "undeclared") and thus hard to understand side effects. Also, this can lead to Spaghetti code.

Duplicate code

As you can see, your functions are pretty much the same, so you can use this and try making a single method. More, we can get rid of that ugly usage of inner methods. It's just not necessary in this case.

What I'd do is:

  • Create three lists: one containing questions, one answers and another one which contains the correct answers.
  • Then create a simple method, and basically do what you've done, but in a shorter / nicer way.

Let's see what we have:

list_of_questions = ['What is the population of New Zealand ?',
                     'What year did the first european set foot on New Zealand (Abel Tasman) ?',
                     'How many Kiwi are there left in New Zealand (Approx) ?',
                     'How many new babys where born in New Zealand in 2015 ?']

lists_of_answers = [['A:1830', 'B:1543', 'C:1642', 'D:1765'],
                    ['A:1830', 'B:1543', 'C:1642', 'D:1765'],
                    ['A:2000', 'B:600',  'C:70,000', 'D:100000'],
                    ['A:61,000', 'B:208,000', 'C:98,000', 'D:18,000']]

list_of_correct_answers = [['D', 'd', '4.5', '4'],
                           ['C', 'c', '1642', '3'],
                           ['D', 'd', '100000', '4'],
                           ['D', 'd', '100000', '4']]


def questions():
    wrong = 0
    right = 0

    for each_question, each_answer, each_correct_answer in zip(list_of_questions, lists_of_answers, list_of_correct_answers):
        print(each_question + '\n' + ' '.join(each_answer))
        get_answer = raw_input()

        if get_answer in each_correct_answer:
            print('Your answer is correct!\n')
            right += 1
        else:
            print('That is not the answer I had in mind!\n')
            wrong += 1
        print('So far, you answered correctly to {0} questions and incorrectly to {1}. Good luck!'.format(right, wrong))


if __name__ == '__main__':
    questions()

You can observe that I also added if __name__ == '__main__':. The reason I did this is that you can also import your code as a module in another script and then run the main function if and when your program decides.

Next steps:

  • Please try to be consistent when writing code.
  • Avoid printing to many things. And if you do so, try to review your grammar.
|improve this answer|||||
\$\endgroup\$
  • 4
    \$\begingroup\$ The answer is good overall but I suggest you avoid 'parallel lists' that must be zipped, a list of triples should be simpler to use and harder to mess up (what happens if one of your lists gets an extra item? All the items after it get de-sinched) \$\endgroup\$ – Caridorc Jul 5 '16 at 11:16
  • 2
    \$\begingroup\$ The triples could even be collections.namedtuple to allow more descriptive access by name (or normal tuples accessed by unpacking) \$\endgroup\$ – Caridorc Jul 5 '16 at 11:20
  • 1
    \$\begingroup\$ Thanks for the tip @Caridorc ^^ I'll add this info later on. \$\endgroup\$ – Grajdeanu Alex. Jul 5 '16 at 11:38
  • \$\begingroup\$ I don't like having the questions in a list and the answers in another. This can lead to confusion if you need to insert a question in the middle. \$\endgroup\$ – njzk2 Jul 5 '16 at 15:18
  • 1
    \$\begingroup\$ @Dex'ter yes, but what bothers me is the separation between questions and answers. I would use a structure to hold the data, to keep related stuff together \$\endgroup\$ – njzk2 Jul 5 '16 at 16:46

Your Answer

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

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