HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Ordering a .txt file numerically by size and alphabetically

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filesizeorderingnumericallyalphabeticallyandtxt

Problem

I am doing a controlled assessment and one of the tasks is that we have to sort the saved pupils scores (numerically), and their names (alphabetically) in a .txt file. Any other improvements to the code would also be greatly appreciated.

```
import random
classcheck = False
while classcheck == False:
classnumber = input("what class are you in")
if classnumber == "1":
print()
classcheck = True
elif classnumber == "2":
print()
classcheck = True
elif classnumber == "3":
print()
classcheck = True
else:
print("that is not a valid class number")

student1name = input("please enter your name")
student1score = 0

for i in range(10):
question1no1 = random.randint(1,20)
question1no2 = random.randint(1,20)

operators = ['+','-','*']
op_number = random.randint(0,2)
op_sym = operators[op_number]
op = op_number

if op == 0:
ans = question1no1 + question1no2
elif op == 1:
ans = question1no1 - question1no2
elif op == 2:
ans = question1no1 * question1no2

print(str(question1no1), str(op_sym) , str(question1no2))

question = "what is "+str(question1no1) + str(op_sym) + str(question1no2)+"?"

student1answer1 = input(question)

print (student1answer1)

if str(student1answer1) == str(ans):
print("congrats you got the answer right")
student1score = student1score + 1

else:
print("sorry you got the answer wrong")

print("your score was " + str(student1score) + " out of 10")

if classnumber == "1":
class1score = open("class1score.txt", "a")
class1score.write("\n" + student1name + (" ") + str(student1score))
class1score.close()

if classnumber == "2":
class2score = open("class2score.txt", "a")
class2score.write("\n" + student1name + (" ") + str(student1score))
class2score.close()

if classnumber == "3":

class3score = open("class3score.t

Solution

My first suggestion would be to use some extra functions. However, let's look at it in several parts.

First, the getting of the class number. You have

classcheck = False
while classcheck == False:
    classnumber = input("what class are you in")
    if classnumber == "1":
        print()
        classcheck = True
    elif classnumber == "2":
        print()
        classcheck = True
    elif classnumber == "3":
        print()
        classcheck = True
    else:
        print("that is not a valid class number")


The if statements are quite the same. By using set-membership, we can reduce the duplicate code.

classcheck = False
while classcheck == False:
    classnumber = input("what class are you in")
    if classnumber in {"1", "2", "3"}:
        classcheck = True
    else:
        print("that is not a valid class number")


Furthermore, writing while foo == False: is not really idiomatic Python. Better would be while foo:. But, in this case you're trying to emulate a do {...} while (...) loop. I'd suggest writing it as follows:

while True:
    classnumber = input("what class are you in")
    if classnumber in {"1", "2", "3"}:
        break
    print("that is not a valid class number")


The asking of the name is quite obvious, let's move to the part where the questions get asked:

for i in range(10):


The variable i does not actually get used. It's a minor nitpick, but convention has it that you should write

for _ in range(10):


instead.

question1no1 = random.randint(1,20)
    question1no2 = random.randint(1,20)


What is question1no1 referring to? This will also be executed for question 2 to 10. Maybe operand_left and operand_right would be better names? Leaving them be for now, but it's something you can ponder about.

Next, the creation of the 'puzzle'/question.

operators = ['+','-','*']
    op_number  = random.randint(0,2)
    op_sym = operators[op_number]
    op = op_number

    if op == 0:
        ans = question1no1 + question1no2
    elif op == 1:
        ans = question1no1 - question1no2
    elif op == 2:
        ans = question1no1 * question1no2


There are a few lines between the definition of the operators, and the calculation of the desired result. First suggestion: use random.choice(operators) instead of random.randint(0, 2)

operators = ['+','-','*']
    op_sym = random.choice(operators)

    if op_sym == '+':
        ans = question1no1 + question1no2
    elif op_sym == '-':
        ans = question1no1 - question1no2
    elif op_sym == '*':
        ans = question1no1 * question1no2


Already a lot clearer, no? Still, I've typed the + sign 3 times here. By using the operator module, I could write:

import operator
...
...

    operators = [
        ('+', operator.add),
        ('-', operator.sub),
        ('*', operator.mul),
    ]
    op_sym, op_func = random.choice(operators)
    ans = op_func(question1no1, question1no2)


Adding a new operator would be a simple method of adding another line in the list above.

I'll assume the explicit print statements are a bit of debugging work, and ignore those. Ideally you'd remove them.

Look at how you write the question.

question = "what is "+str(question1no1) + str(op_sym) + str(question1no2)+"?"


There are so many things going on, it is a bit worry-some. Also, you might want to add a space around the operator, causing the need for yet some more work. By using string formatting (https://docs.python.org/3.5/library/stdtypes.html#str.format), you can make it a bit simpler:

question = "what is {} {} {}?".format(question1no1, op_sym, question1no2)


Now, we get to student1answer1. Why not just answer? (Or given_answer, and rename ans to expected_answer).

As for checking the results:

if str(student1answer1) == str(ans):
        print("congrats you got the answer right")
        student1score = student1score + 1

    else:
        print("sorry you got the answer wrong")


You can remove the empty line between the if and the else block. But more importantly, instead of

student1score = student1score + 1


you can write

student1score += 1


with the same effect.

At the end, we also see duplicated code regarding to the class numbers:

if classnumber == "1":
    class1score = open("class1score.txt", "a")
    class1score.write("\n" + student1name + (" ") + str(student1score))
    class1score.close()

if classnumber == "2":
    class2score = open("class2score.txt", "a")
    class2score.write("\n" + student1name + (" ") + str(student1score))
    class2score.close()

if classnumber == "3":

    class3score = open("class3score.txt", "a")
    class3score.write("\n" + student1name + (" ") + str(student1score))
    class3score.close()


It should be quite obvious that the only difference in these statements is the filename. "Easy" fix:

```
if classnumber == "1":
filename = "class1score.txt"
if classnumber =

Code Snippets

classcheck = False
while classcheck == False:
    classnumber = input("what class are you in")
    if classnumber == "1":
        print()
        classcheck = True
    elif classnumber == "2":
        print()
        classcheck = True
    elif classnumber == "3":
        print()
        classcheck = True
    else:
        print("that is not a valid class number")
classcheck = False
while classcheck == False:
    classnumber = input("what class are you in")
    if classnumber in {"1", "2", "3"}:
        classcheck = True
    else:
        print("that is not a valid class number")
while True:
    classnumber = input("what class are you in")
    if classnumber in {"1", "2", "3"}:
        break
    print("that is not a valid class number")
for i in range(10):
for _ in range(10):

Context

StackExchange Code Review Q#117271, answer score: 3

Revisions (0)

No revisions yet.