Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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
share|improve this question
3  
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. –  Jamal Jul 27 at 5:54
    
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. –  Tom Hart Jul 28 at 10:08

4 Answers 4

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.

share|improve this answer
    
Ah I knew my naming seemed weird compared to code I've seen. Thanks every bit of info helps! –  Dzzs Jul 27 at 4:45
2  
Note that Python has a coding convention known as PEP8. Variables should be named like_this (so user_input, for example). –  Yuushi Jul 27 at 5:21
1  
that is okay , as long as you have better name or which ever is useful to improve readability of code. –  paritosh Jul 27 at 5:27
1  
@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. –  Pharap Jul 27 at 10:29
2  
@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. –  Yuushi Jul 27 at 11:14

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
share|improve this answer
  • 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.

share|improve this answer

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
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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