5
\$\begingroup\$

A program that generates questions to train your number base conversion skills. This program is made to ask questions about octet, binary and hexadecimal. No other bases. It first asks a series of questions to the user such as "amount of questions", "question type" and "base". Then, it starts generating questions based on the user's input. The user can give an answer and it will tell them if they're correct. Code is perfectly functional.

I'm asking for a review because I sense a bad smell in my code. It feels overly long, repetitive, and kind of confusing. There surely must be a better way. Maybe an OOP approach? Something to cut down on the complexity and "if-else" repetition. Though, I feel it is this way partly because I have limited it to only 3 bases. I'm a late beginner but I wouldn't say that I am intermediate just yet.

My code uses only one external library, that is baseconvert. Can be installed with pip using pip install baseconvert

import random
import baseconvert


# The Start
# ---------
# Step 1: Ask whether the user wants 'Decimal > Base X' or 'Base X > Decimal' or 'Random'
# Step 2: Ask how many questions the user wants. Type the answer. Type -1 for infinite.
# Step 3: If infinite, prompt to type 'f' to stop.
# Step 4: Ask what base you want based on index. 1. Octet // 2. Hexadecimal // 3. Binary // 4. Random

def question_type_ask():
    print('Which question would you like to be asked?')
    print('1. Decimal > Base X\n2. Base X > Decimal\n3. Random')
    ans = input('Input: ')
    if ans != '1' and ans != '2' and ans != '3':
        print('Invalid input')
        question_type_ask()
    return ans


def amount_ask():
    print('How many questions do you want to be asked? Type -1 for infinite.')
    print('Type \'f\' as answer to stop')
    try:
        ans = int(input('Input: '))
    except ValueError:
        print('Input numbers only, please.')
        return amount_ask()
    if ans >= -1:
        return ans
    else:
        return amount_ask()


def base_ask():
    print('Which base do you want to be asked?')
    print('1. Octet\n2. Hexadecimal\n3. Binary\n4. Random')
    ans = input('Input: ')
    if ans != '1' and ans != '2' and ans != '3' and ans != '4':
        print('Invalid input')
        base_ask()
    return ans


question_type = question_type_ask()
amount = amount_ask()
base = base_ask()
is_random_question = False
is_random_base = False

if question_type == '3':
    is_random_question = True
if base == '4':
    is_random_base = True


def question_generator():
    global question_type
    global amount
    global base

    base_list = ['1', '2', '3']
    type_list = ['1', '2']

    while True:
        if is_random_base:
            base = random.choice(base_list)
        if is_random_question:
            question_type = random.choice(type_list)
        if question_type == '1':  # Decimal > Base X
            base1 = 'Base 10'
            num = random.randrange(100, 100000)
            if base == '1':  # Octet
                base2 = 'Base 8'
                correct_ans = baseconvert.base(num, 10, 8, string=True)
                break
            elif base == '2':  # Hexa
                base2 = 'Base 16'
                correct_ans = baseconvert.base(num, 10, 16, string=True)
                break
            elif base == '3':  # Binary
                base2 = 'Base 2'
                correct_ans = baseconvert.base(num, 10, 2, string=True)
                break
            elif base == '4':  # Random
                base2 = random.choice(base_list)
                if base2 == base_list[0]:
                    base = '1'
                elif base2 == base_list[1]:
                    base = '2'
                elif base2 == base_list[2]:
                    base = '3'
        elif question_type == '2':  # Base X > Decimal
            base2 = 'Base 10'
            correct_ans = str(random.randrange(100, 100000))
            if base == '1':  # Octet
                base1 = 'Base 8'
                num = baseconvert.base(correct_ans, 10, 8, string=True)
                break
            elif base == '2':  # Hexa
                base1 = 'Base 16'
                num = baseconvert.base(correct_ans, 10, 16, string=True)
                break
            elif base == '3':  # Binary
                base1 = 'Base 2'
                num = baseconvert.base(correct_ans, 10, 2, string=True)
                break
            elif base == '4':  # Random
                base1 = random.choice(base_list)
                if base1 == base_list[0]:
                    base = '1'
                elif base1 == base_list[1]:
                    base = '2'
                elif base1 == base_list[2]:
                    base = '3'

    print(f'[{base1}] {num} to [{base2}]')
    ans = input('Answer: ')
    if ans == correct_ans:
        print('You are correct!')
    elif ans.lower() == 'f':
        question_generator()
    else:
        print(f'Wrong! The answer is {correct_ans}')


counter = 0
if amount == -1:
    amount = float('inf')
while counter < amount:
    counter += 1
    question_generator()

\$\endgroup\$
  • \$\begingroup\$ New code scored 9.05/10 in pylint so that's awesome. Just one question, is the while True: loop inside question_generator function really necessary ? \$\endgroup\$ – impopularGuy May 14 at 11:53
  • \$\begingroup\$ I have rolled back your latest post. Once an answer has been provided, you may not change your question. See the help center, specifically the What should I do when someone answers my question?, and especially "What should I not do". If you want your updated code to be reviewed, you must post a new question. \$\endgroup\$ – AJNeufeld May 14 at 14:45
  • \$\begingroup\$ You can post a new question or simply discuss more in chat \$\endgroup\$ – impopularGuy May 14 at 15:49
2
\$\begingroup\$
  • Firstly I would recommend not to use recursive methods for handling wrong inputs. A simple while loop is enough.
  • Use .strip() while taking inputs since 'a'=='a ' is False in python.
  • If you are playing with numbers, keep them as numbers, not as strings. Usually numbers are easier to handle.
  • Avoid use of global if simple passing as arguments is an available option.
  • In the if-elif ladder, the condition base=='4' is unnecessary since you are already handling random case using the flags is_random_question and is_random_base.
  • I have added a dictionary base_dict which eliminates the need of if-else repetitions.
  • Why would you do elif ans.lower()=='f': question_generator() when you want 'Type \'f\' as answer to stop'? I have modified the while loop inside generate_question method to work as counter for number of questions.
import random
import baseconvert

def question_type_ask():
    print('Which question would you like to be asked?')
    print('1. Decimal > Base X\n2. Base X > Decimal\n3. Random')
    ans = input('Input: ').strip()
    # print(ans)
    while ans<'1' or ans>'3':
        ans = input('Invalid input. Enter again: ')
    return int(ans)

def amount_ask():
    print('How many questions do you want to be asked? Type -1 for infinite.')
    print('Type \'f\' as answer to stop')
    while True:
        ans = input('Input: ').strip()
        if not ans.lstrip('-+').isnumeric():
            print('Input numbers only, please.')
        elif int(ans)<-1 or int(ans)==0:
            print('Input -1 or positive numbers only, please.')        
        else:
            break
    return int(ans)

def base_ask():
    print('Which base do you want to be asked?')
    print('1. Octet\n2. Hexadecimal\n3. Binary\n4. Random')
    ans = input('Input: ').strip()
    # print(ans)
    while ans<'1' or ans>'4':
        ans = input('Invalid input. Enter again: ')
    return int(ans)

question_type = question_type_ask()
amount = amount_ask()
base = base_ask()
# print(question_type, amount, base)
is_random_question = False
is_random_base = False
if question_type == 3:
    is_random_question = True
if base == 4:
    is_random_base = True

def question_generator(question_type, amount, base):
    base_list = [1, 2, 3]
    type_list = [1, 2]
    base_dict = {1:8, 2:16, 3:2}

    counter = 0
    if amount == -1:
        amount = float('inf')
    while counter < amount:
        counter += 1

        if is_random_base:
            base = random.choice(base_list)
        if is_random_question:
            question_type = random.choice(type_list)
        if question_type == 1:  # Decimal > Base X
            num = random.randrange(10, 100)
            base1 = 10
            base2 = base_dict[base]
            correct_ans = baseconvert.base(num, base1, base2, string=True)
        elif question_type == 2:  # Base X > Decimal
            base1 = base_dict[base]
            base2 = 10
            correct_ans = str(random.randrange(10, 100))
            num = baseconvert.base(correct_ans, base2, base1, string=True)

        print(f'\n[Base {base1}] {num} to [Base {base2}] ?')
        ans = input('Answer: ').strip()
        if ans.lower() == 'f':
            break
        if ans == correct_ans.lower():
            print('You are correct!')
        else:
            print(f'Wrong! The answer is {correct_ans}')

question_generator(question_type, amount, base)
```
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you! Excellent answer. The idea of using dictionaries had never crossed my mind. \$\endgroup\$ – Harris May 13 at 7:04
  • \$\begingroup\$ I've completely rewritten my code based on your suggestions. Can you take a look and tell me what you think? \$\endgroup\$ – Harris May 14 at 4:31
  • \$\begingroup\$ You use .strip() when getting the first input (ans = input('Input: ').strip()) but not when you ask the user to re-enter (ans = input('Invalid input. Enter again: ')). The test ans<'1' or ans>'3' is broken, since "2048" will pass, the first character being greater than "1" and less than "3". Also, not ans.lstrip('-+').isnumeric() is not sufficient validation as "-+-+23" will pass. Also, "¼⁸".isnumeric() returns True; the method you want is .isdecimal(). \$\endgroup\$ – AJNeufeld May 14 at 14:54
  • \$\begingroup\$ @AJNeufeld Thanks for pointing out such details that i somehow missed. If i remember correctly, the new code (not there anymore) did solve some of the points you mentioned. \$\endgroup\$ – impopularGuy May 14 at 15:48
2
\$\begingroup\$

When you ask the user to input: (It applys in question_type_ask(), ask_amount() and ask_base())

It's better to add .strip() after taking the input; otherwise, the input with leading whitespace will be invalid.

You may use try...except to check the input's validity:

while True:
        try:
            ans = int(ans)
            if ans < 1 or ans > 3:
                ans = input('Invalid input. Try again: ').strip()
            else: 
                return ans
        except ValueError: 
            ans = input('Invalid input. Try again: ').strip()

question_generator(question_type, amount, base):

base_dict = {1: 8, 2: 16, 3: 2, 4: 'Random'} ('Random' or None) is better; because in Python, all non-zero integers are True.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Ah, yes! I forgot about .strip(). That last fact about non-zero integers is very helpful. It explains why I had to use stored_base is True: instead of just stored_base:. Thanks! \$\endgroup\$ – Harris May 14 at 9:05

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.