25
\$\begingroup\$

I have been wanting to learn programming for some time now and decided to really give it a shot today. I have edited and read lots of things in Java to fix small issues but never written from scratch. I've tried C and Java but both seemed just a bit too much to teach myself as a first language, so I'm going with Python first.

Can anyone look through this little sample random number generator I made and point out anything I should be doing differently or correctly? It is executing without issues for me but I'm sure I've got extra nonsense in here. I just have all the prints to see a result of each step and I know this is not what a real random number generator would be like. I'm just testing and learning my way around different aspects of the language.

import random


x = int(raw_input("Please enter a number up to 100. "))
z = random.randint(40,100)
""" x and z can both be used below without errors -- testing/learning"""

randomNumber = random.randint(1,z)
print "first random",randomNumber


if 35 >= randomNumber:
    newRandom = randomNumber * .5
    newRandom = newRandom + 9
    print "second random " "%.f" % newRandom

elif 36 <= randomNumber <= 93:
    newRandom = randomNumber + 7
    print "second random " "%.f" % newRandom

else:
    newRandom = randomNumber
    print "second random",newRandom
\$\endgroup\$
2
  • 4
    \$\begingroup\$ Do you actually do anything with the user input (x is initialized but never used)? If so, please include that aspect of the code without making any changes already mentioned in the answers. \$\endgroup\$
    – Jamal
    Jul 27, 2014 at 5:54
  • \$\begingroup\$ I believe the comment suggests that they are toggling random.randint(1,x|z) for testing, so whilst I can't seeing it being used in the code above, it read it as it could be used there. \$\endgroup\$
    – TMH
    Jul 28, 2014 at 10:08

5 Answers 5

16
\$\begingroup\$

Code organization

Rather than having your code in the global namespace, it's better to wrap it into a method, for example:

def gen_random_number():
    randomNumber = random.randint(1, z)
    print "first random", randomNumber
    # ...

And call the method like this:

if __name__ == '__main__':
    gen_random_number()

Magic numbers

Avoid using "magic numbers": the numbers 40, 100, 35, 93 are "magic" because it's unclear what they are and why. If you put them in capitalized constants near the top of the file with a good name, it becomes much clearer what they are and their purpose, for example:

LOWER_BOUND = 40
UPPER_BOUND = 100

z = random.randint(LOWER_BOUND, UPPER_BOUND)

For 40 and 100 it was easy enough to guess what they are and give these names. But for 35, 93 I'm not sure. If you had given them a name, I wouldn't have to wonder.

Formatting

There is an official standard Python style guide called PEP8. I suggest to read and follow that, it will make it easier for others to review your code. There is command line utility called pep8 that can detect coding style violations. You can install it with pip install pep8, and run it on your script (or even an entire directory) simply with pep8 path/to/your/code.

Your code reformatted and variables renamed to follow PEP8 would look like this:

random_number = random.randint(1, z)
print "first random", random_number

if 35 >= random_number:
    new_random = random_number * .5
    new_random += 9
    print "second random " "%.f" % new_random

elif 36 <= random_number <= 93:
    new_random = random_number + 7
    print "second random " "%.f" % new_random

else:
    new_random = random_number
    print "second random", new_random

Although PEP8 doesn't complain about this:

print "second random " "%.f" % new_random

There's no need to put a space between those two strings, so you can write it simply:

print "second random %.f" % new_random

You could further simplify this:

new_random = random_number * .5
new_random += 9

to this:

new_random = random_number * .5 + 9
\$\endgroup\$
12
\$\begingroup\$

As @Jamal pointed out, the input prompt is unclear, and in any case x is never used.

Put a space after each comma for readability.

The code would be easier to understand with all inequalities pointing in the same direction. Comparison with 36 is unnecessary.

In the ≤ 35 case, why not assign newRandom correctly the first time?

The printout is common to all three code branches, and should be factored out.

…

if randomNumber <= 35:
    newRandom = randomNumber * .5 + 9
elif randomNumber <= 93:
    newRandom = randomNumber + 7
else:
    newRandom = randomNumber
print "second random %.f" % newRandom
\$\endgroup\$
11
\$\begingroup\$

Variable-naming is one of the important aspects of coding. You must name variables correctly; in this case, x and y do not make sense.

Rather, it should be:

userInput = int(raw_input("Please enter a number up to 100. "))
selectedRandomNumber= random.randint(40,100)

The rest is okay as part of learning.

\$\endgroup\$
5
  • \$\begingroup\$ Ah I knew my naming seemed weird compared to code I've seen. Thanks every bit of info helps! \$\endgroup\$
    – Dzzs
    Jul 27, 2014 at 4:45
  • 3
    \$\begingroup\$ Note that Python has a coding convention known as PEP8. Variables should be named like_this (so user_input, for example). \$\endgroup\$
    – Yuushi
    Jul 27, 2014 at 5:21
  • 1
    \$\begingroup\$ that is okay , as long as you have better name or which ever is useful to improve readability of code. \$\endgroup\$
    – Paritosh
    Jul 27, 2014 at 5:27
  • 1
    \$\begingroup\$ @Yuushi I'd like to emphasise the fact that the page in question is a 'guide'. I tend to think of guides as suggestions rather than absolutes. \$\endgroup\$
    – Pharap
    Jul 27, 2014 at 10:29
  • 3
    \$\begingroup\$ @Pharap It's a guide, but one that is used by a huge majority of Python code out there. Unless there is a really good reason not to, it's a guide that you should stick to. \$\endgroup\$
    – Yuushi
    Jul 27, 2014 at 11:14
10
\$\begingroup\$
  • This prompt isn't entirely clear:

    int(raw_input("Please enter a number up to 100. "))
    

    Does that mean any number before 100, including negative numbers? The user won't know the proper minimum unless it's stated. Either way, you should also have input validation to make sure the program will only proceed after receiving proper input from the user.

  • If a value will be accumulated:

    newRandom = newRandom + 9
    

    you can just use the += operator:

    newRandom += 9
    

    This also works with other operators.

\$\endgroup\$
1
\$\begingroup\$

In addition to the other answers, I'd like to point out that your program could crash if someone does not enter a number in the raw_input.

I recommend you surround it with a try/except block like this (please read here for more explanation about try/except):

try:
    x = int(raw_input("Please enter a number up to 100.\n"))
except:
    stuff_to_do_if_exception_is_raised()

For example, you could use a loop and do:

while True:
    try:
        x = int(raw_input("Please enter a number up to 100.\n"))
        if x>100:
            print "Number is too large"
        else:
            break
    except (ValueError, NameError, SyntaxError):
        print "This is not a number"
\$\endgroup\$

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