9

Original Question

I have made a function which is waiting for a particular string to appear on a serial port, and returns all character read until the string was found, or false if not. This is quite convenient, but I was wondering if it is considered bad practice or not ?

Clarification :

The primary goal is to wait for a particular string to appear in a given amount of time. Except for IO error, the possible outcome is True (the string did appear) or False The secondary goal is to get the entire output, because there might be information that I would like to parse for before the actual answer that is looked for. I thought may be I could combine the primary and secondary goal in one return value.

def MyFunc(s, timeout) :
    test = get_some_input(timeout)
    if test.endswith(s)
        return test
    else
        return False

Edit : Another proposed answer is to raise an exception. I don't think it is a good idea, because the timeout is an expected behaviour. I mean, if there is a parameter for specifying a timeout, then a timeout is a possible outcome, and not an exception.

Edit 2 : Since I need to store the input, maybe using a class is the right solution. The wait for function has a clear return value, yet the entire string that was read until timeout is also accessible.

class Parser :
        def __init__(self, sport_name):
                self.currentMsg = ''
                self.ser = serial.Serial(sport_name, 115200)
        def WaitFor(self, s, timeOut=None):
                self.ser.timeout = timeOut
                self.currentMsg = ''
                while self.currentMsg.endswith(s) != True :
                        # should add a try catch here
                        c=self.ser.read()
                        if c != '' :
                               self.currentMsg += c
                        else :
                                print 'timeout waiting for ' + s
                                return False
                return True

8 Answers 8

28

Would it not be more suitable to return a None instead of False?

3
  • but as described in other answers, an exception is even more Pythonic. Mar 18, 2009 at 13:33
  • An exception isn't Pythonic in this case. Look at the behavior of the standard library -- in particular select.select and re.match. Mar 18, 2009 at 13:49
  • I think both solution are good: returning None or raising an exception. It depends on the overall way you handle failure or abnormal conditions in your program.
    – Ber
    Mar 19, 2009 at 8:15
12

I believe the orthodox Python design would be to return None. The manual says:

None

This type has a single value. There is a single object with this value. This object is accessed through the built-in name None. It is used to signify the absence of a value in many situations, e.g., it is returned from functions that don’t explicitly return anything. Its truth value is false.

8

It would be better to return a string AND a boolean (as in the title) instead of returning a string OR a boolean. You shouldn't have to figure out what the return value means. It should be totally explicit and orthogonal issues should be separated into different variables.

(okay,value) = get_some_input(blah);
if (okay): print value

I tend not to return tuples a lot, because it feels funny. But it's perfectly valid to do so.

Returning "None" is a valid solution, already mentioned here.

5
  • That orthogonal thing is perfectly right. So returning a tuple would be a good solution, however I am not yet very comfortable with this, plus using a class allows me to embed a serial port in it, and allows for further extension.
    – shodanex
    Mar 19, 2009 at 9:08
  • so, what should be the value if okay is False? Mar 19, 2009 at 9:13
  • @SilentGhost: value is always an input read so far (until string found or timeout occurred).
    – jfs
    Mar 20, 2009 at 20:27
  • if okay is false, value could be an empty string or whatever input that came until timeout occured
    – shodanex
    Mar 22, 2009 at 15:14
  • If okay is false, the value is undefined. Could be None. Could be 42. I kinda like None. Apr 9, 2009 at 12:43
6

The convenient thing is to return an empty string in this case.

Besides an empty string in Python will evaluate to False anyway. So you could call it like:

if Myfunc(s, timeout):
    print "success"

Addition: As pointed out by S.Lott the true Pythonic way is to return None. Though I choose to return strings in string related funcs. A matter of preference indeed.

Also I assume the caller of Myfunc only cares about getting a string to manipulate on - empty or not. If the caller needs to check about timeout issues, etc.. it's better to use exceptions or returning None.

9
  • The trouble with this design is that it doesn't distinguish between timeout in one case, and s and test being equal to "". Mar 18, 2009 at 13:01
  • -1: Not very Pythonic at all. Empty strings are still strings. None is better. An exception is better still.
    – S.Lott
    Mar 18, 2009 at 13:09
  • @utku karatas: Not a matter of preference -- it's a matter of meaning. A zero-length string means you read zero until delimiter or end-of-file. A None means you did not read.
    – S.Lott
    Mar 18, 2009 at 13:49
  • @S.Lott: I assume the case where the caller of Myfunc only cares about getting a string to manipulate on - empty or not. Mar 18, 2009 at 14:15
  • @utku karatas: string is one thing -- timeout is not a string. It remains a matter of meaning.
    – S.Lott
    Mar 18, 2009 at 15:21
4

Maybe if you return a tuple like (False, None) and (True, test) it would be better, as you can evaluate them separatedly and not add unnecesary complexity.

EDIT: Perhaps the string that appeared on the serial port is "" (maybe expected), so returning True can say that it arrived that way.

3
  • I'd rather go the @jelovirt way: testing for None is unambiguous and does not really add complexity. Mar 18, 2009 at 12:46
  • I've always like the python tuples for returning multiple values. +1.
    – paxdiablo
    Mar 18, 2009 at 12:55
  • I like this idea, returning a control flag in the same value field doesn't sound right to me. In that kind of situations, you have the risk that in the future the valid values change (like you said, allowing null) and the control flag become invalid.
    – Sam
    Mar 18, 2009 at 15:27
4

You could return the string if it arrived in time, or raise a suitable exception indicating time out.

3
  • Even if raising an exception in Python is inexpensive compared to other languages, I don't think that providing functionality using them is a great idea.
    – Martin
    Mar 18, 2009 at 12:52
  • +1: Exceptions make more sense in this case -- you had an "exception" condition -- a timeout.
    – S.Lott
    Mar 18, 2009 at 12:58
  • In fact I have used this pattern a lot and very successfully in a project that uses communication via serial ports etc. The time out exception handles retries, resend conditions quite well.
    – Ber
    Mar 18, 2009 at 14:53
2

To add to Ber's point, you might want to take something else into account. If you use an empty string or None, you leave the door open for bugs of the "dumb" variety. On the other hand, if you raise an exception, you're forcing the execution of whatever operation is running to be aborted.

For example, consider the following code:

result = MyFunc(s, timeout)
if result[0] == 'a':
    do_something()

This will raise an exception if the operation timed out and got either an empty string or None. So you'd have to change that to:

result = MyFunc(s, timeout)
if result and result[0] == 'a':
    do_something()

These kinds of changes tend to add up and make your code more difficult to understand.

Of course, I'm sure that your answer to this will be something along the lines of "I won't do that" or "that won't happen" to which my answer is "Even if you don't run into it with this function, you will eventually if you make a habit of doing this." These kinds of bugs are almost always the result of corner cases that you don't usually think about.

1

This is a classic use case for Python generators. The yield keyword provides a simple way to iterate over discrete sets without returning the whole thing at once:

def MyFunc(s, timeout) :
    test = get_some_input(timeout)
    while test.endswith(s)
        yield test
        test = get_some_input(timeout)

for input in MyFunc(s, timeout):
    print input

The key here is there is no return value to specify the end of input; instead, you simply reach the end of the iterator. More information on generators here.

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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