0

I am writing a small function that takes an integer value and a string such as:

param1: 1
param2: "1 1 1"

The function will split the string parameter and validate its len against the first parameter like so:

def prepare_integer_set(expected_len, integer_string):
    prepared_integer_set = integer_string.split()
    if len(prepared_integer_set) != expected_len:
        raise ValueError('Number of expected integers does not match integer string')
    return [int(x) for x in prepared_integer_set]

However, the exception is bothering me. I would rather return false as the input comes from the user so is not strictly an exception if they make an error.

How should this be handled? Split the method in 2, having a separate validation and preparation methods? Or is it pythonic as it currently is by throwing the exception?

Here would be the alternative which is split:

def validate_integer_set(expected_len, integer_set):
    return expected_len == len(integer_set)


def prepare_integer_set(integer_string):
    prepared_integer_set = integer_string.split()
    return [int(x) for x in prepared_integer_set]
3
  • 2
    If you want to return False why not just return False? Commented Feb 17, 2015 at 21:43
  • 1
    Because i will then be returning mixed types Commented Feb 17, 2015 at 21:44
  • So? This is not necessarily a bad thing... It depends on what this function does (in the context of the application) and where it does it ... Commented Feb 17, 2015 at 21:46

2 Answers 2

2

You need to decide what prepare_integer_set() is doing:

  • If it is just validating user input, it should return True or False. If it returns True, you then process the data as normal.
  • If it is doing something with the data, then invalid data ought to result in an exception.
  • You could alternatively return None or some other falsey value, but be careful. If your normal return value could also be falsey (e.g. an empty list), then this is likely to cause more trouble than it's worth.
Sign up to request clarification or add additional context in comments.

7 Comments

an empty list could not be possible if the length test passed
@PadraicCunningham: param1 = 0 and param2 = ''. The .split() will return an empty list.
0 an an empty string would make the function rather redundant
or not prepared_integer_set: in the if check will catch 0 and ""``, the fact that "1 foo"` will raise an error trying to cast to an int makes me think either both should return False for invalid input or both should raise an exception, by not using try/except when casting is not really validating input.
without knowing exactly what the OP wants to do with the output it is hard to give a definitive answer but for me anyway it looks like a case for a try except either returning False for invalid input or raising an appropriate error message for the user in both cases.
|
0

I would return False if you don't want to raise an error then use a simple if check later before proceeding:

def prepare_integer_set(expected_len, integer_string):
    prepared_integer_set = integer_string.split()
    # catch 0 and negative input
    if len(prepared_integer_set) != expected_len or expected_len  < 1:
        return False
    return [int(x) for x in prepared_integer_set]

val = prepare_integer_set(2,"1 2")
if val: # if length test failed it will evaluate to False else we have a list of ints which will evaluate to True
    ...

You also have to consider input like, prepare_integer_set(3,"1 2 foo") that will pass your current length test but fail when casting. So you may want to use a try/except returning False again.

def prepare_integer_set(expected_len, integer_string):
    prepared_integer_set = integer_string.split()
    if len(prepared_integer_set) != expected_len or expected_len  < 1:
        return False
    try:
        return [int(x) for x in prepared_integer_set]
    except ValueError:
        return False

If you don't consider the wrong length to be an error then input that cannot be cast to an int should probably be treated the same or both should be considered an error and an exception raised instead of returning False. Really what you are doing with the value returned should dictate whether you should raise an error or return False for bad input, I don't think doing different things for both situations is a good idea.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.