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.
str, ski = int(input("text here")), int(input("text here"))

    if str < 1 or str > 50 or ski < 1 or ski > 50:
        print("did not enter between 1 and 50")

Is there anyway to shorten the if statement in that code? I find that it is a bit too long, but I don't really know how to shorten it.

Any suggestions?

share|improve this question

closed as off-topic by 200_success Dec 4 at 20:02

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

1  
When asking a question, please make an effort to choose a good title that tells the community specifically what the point of the code in question is. I have edited the title to this effect. –  codesparkle Feb 21 at 0:10

3 Answers 3

up vote 7 down vote accepted
  • This snippet is not very useful. You should not let the user continue after giving you wrong input; rather, as soon as an incorrect number is entered, you should figure out what to do next.
    (As you have not included any context in your question, this is left as an exercise to you).

  • Your names are extremely poor:
    str is a library function and should therefore never be used as a variable name.
    str and ski are useless as they do not reveal what the variables are about.
    Be expressive.

  • You are repeating yourself on a small scale: you are writing int(input(...)) twice and the two portions of the if statement follow the same logic. Get rid of the duplication.

Consider these improvements applied to your code:

  1. Use list comprehensions and generator expressions to your advantage
  2. Use the all(...) function with a generator expresssion to verify that all answers are in the specified range

prompts = ('text here', 'text here')
answers = [int(input(p)) for p in prompts]
if not all(0 < a <= 50 for a in answers):
    print('did not enter between 1 and 50')

# if you need to access the answers individually, use
first, second = answers
share|improve this answer

You could do something like this:

attributes = {}
for attribute in ["STR", "PER", "END", "CHA", "INT", "AGI", "LCK"]:
    input_value = 0
    while not 1 <= input_value <= 50:
        print "Please enter a value between 1 and 50 (inclusive)."
        try:
            input_value = int(input("Enter %s: " % attribute))
        except ValueError:
            pass

    attributes[attribute] = input_value
share|improve this answer

+1 @instanceofTom and you could remove some more duplication and create a more readable code with a named function which would tell the intent of the condition:

def is_valid_property(property):
    return 1 <= property <= 50

if (not is_valid_property(strength) or not is_valid_property(ski)):
    print("<> please write numbers between 1 and 50")

(Feel free to rename it, I'm just guessing about the intent.)

Note the changed message too. Error messages shouldn't blame the user (don't say what he did is wrong), give a suggestion about what they should do. (See: Should we avoid negative words when writing error messages?, What will be the Best notifications and error messages?)

share|improve this answer
    
This answer can't work for the same reason as the other answer, and in addition ! is a syntax error in Python. –  Gareth Rees Feb 20 at 23:14
    
@GarethRees: Thanks, fixed. –  palacsint Feb 21 at 0:04

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