current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 5 down vote favorite

I'm in a location with poor Internet access. In order to ensure that a call to an external service succeeds, I am wrapping the call in a pseudo-do-while in Python. Additionally, I am limiting the call to running a maximum of three times:

def post_safe(url, params):

    done = 0
    max_tries = 3
    messages = []

    while done<max_tries:
        try:
            response = requests.post(url, data=params)
        except Exception as e:
            messages.append(e)
            time.sleep(1)
        else:
            done = max_tries+1
        done += 1

    if done==max_tries:
        output = "%s\n" % (datetime.now().strftime('%Y-%m-%d %H:%M'),)
        output+= "requests() failed 3 times:\n"
        for m in messages:
            output+= m+"\n"
        print(output)

        return False

    return True

I personally am not satisfied with the 'Pythoness' of this code. It looks inelegant, and I would have preferred to use the type bool for done but that would require a second variable to count the iterations.

Is there a cleaner way to achieve a do-while in Python while limiting the amount of iterations?

share|improve this question
up vote 1 down vote accepted

The else branch in your try is hackish (skipping over the max_tries) so I added success variable.
(Although you asked not to avoid it in the question, I think it's the better way, and I'll explain why.)

It also nicely allowed me to

  • replace if done==max_tries with if not success;
  • remove else branch altogether;
  • return success instead of having two return statements.

I think it reads more easily now.

def post_safe(url, params):
    messages = []

    success = False
    max_retries = 3
    retries = 0

    while retries < max_retries and not success:
        try:
            response = requests.post(url, data=params)
            success = True
        except Exception as e:
            messages.append(e)
            time.sleep(1)

        retries += 1

    if not success:
        output = "%s\n" % (datetime.now().strftime('%Y-%m-%d %H:%M'),)
        output+= "requests() failed 3 times:\n"
        for m in messages:
            output+= m+"\n"
        print(output)

    return success

You can also get rid of success by returning True early in try:

def post_safe(url, params):
    messages = []

    retries = 0
    max_retries = 3

    while retries < max_retries:
        try:
            response = requests.post(url, data=params)
            return True
        except Exception as e:
            messages.append(e)
            time.sleep(1)

        retries += 1

    output = "%s\n" % (datetime.now().strftime('%Y-%m-%d %H:%M'),)
    output+= "requests() failed 3 times:\n"
    for m in messages:
        output+= m+"\n"
    print(output)

    return False

I don't like this approach. It wins us a variable and two lines but obfuscates the flow:

  • In which case will the last block run? When we had an if not success condition, it was obvious, but now from a quick glance it looks like this block executes every time;
  • What are function's return points? The first example had just one, and it was obvious what the return value represented.

Let me quote PEP 20:

  • Sparse is better than dense.
  • Readability counts.

Finally, success may be an “extra” variable from computer's point of view, but for a human, it's the expression that determines if the last block of code runs. It's good to give it a name.

share|improve this answer
1  
Thanks, Dan. Though just a tad more readable, I don't like the use of the extra variable (I mention that in the OP). I do think that the extra readability is due to the better-named variable (done was a terrible name). I'm hoping that a more 'Pythonic' answer comes along. I would almost expect this to be a common design pattern. – dotancohen Jan 7 '14 at 16:37
    
@dotancohen I added a second example that doesn't need success, but personally I find it less obvious. – Dan Jan 7 '14 at 16:42
    
There were some terrific answers here. I choose this answer based on the readability. It feels wasteful, but it is very readable and maintainable. Thank you! – dotancohen Jan 8 '14 at 12:39
up vote 8 down vote

Python has a language feature just for that: else clauses on loops.

Loop statements may have an else clause; it is executed when the loop terminates through exhaustion of the list (with for) or when the condition becomes false (with while), but not when the loop is terminated by a break statement.

def post_safe(url, params):
    max_tries = 3
    messages = []

    for _ in range(max_tries):
        try:
            response = requests.post(url, data=params)
            return True
        except Exception as e:
            messages.append(e)
            time.sleep(1)

    else:
        print(datetime.now().strftime('%Y-%m-%d %H:%M'))
        print("requests() failed %d times:" % (max_tries))
        for m in messages:
            print(m)
        return False

I'd also make the following changes:

  • The else clause on the try block is unnecessary; it can be replaced with a break within the try block just after calling .post(). But in the end, you'll just end up returning True, so why not get out of there right away with return True? (For that matter, the else block that I just suggested is also unnecessary, as the else could be removed and its contents outdented by one level.)
  • It seems easier to print each line separately than to build a multi-line string to print all at once. You might want to consider logging instead.
share|improve this answer
    
huh ... lsned - learn something new every day. – rolfl Jan 7 '14 at 17:47
1  
Considering the return True in the try block, the else clause on the for loop is unnecessary too! But I must say, for-else is great and thank you for mentioning it. – dotancohen Jan 7 '14 at 18:36
up vote 3 down vote

One of the beauties of Python is that it can be made to read like English. With a very small amount of messing with your code, you can get to this, which I feel is much more readable:

def post_safe(url, params):

    done = False
    tries_left = 3
    messages = []

    while tries_left and not done:
        tries_left -= 1
        try:
            response = requests.post(url, data=params)
            done = True
        except Exception as e:
            messages.append(e)
            time.sleep(1)

    if not done:
        output = "%s\n" % (datetime.now().strftime('%Y-%m-%d %H:%M'),)
        output+= "requests() failed 3 times:\n"
        for m in messages:
            output+= m+"\n"
        print(output)

    return done
share|improve this answer
    
Great idea about decrementing the counter. – Dan Jan 7 '14 at 17:08
2  
That's what optimization freaks teach you to do in C: it seems that comparing to 0 is cheaper in some computer architectures... – Jaime Jan 7 '14 at 17:09
    
Thank you, I also like the idea of decrementing the counter. I'll deliberate a bit before choosing one of the two excellent answers. Maybe a third will show up in the meantime! – dotancohen Jan 7 '14 at 17:25
    
nit-pick - sometimes I like val += 1 instead of val++ because it makes sense in that context, but, because - and = look more alike, I never recommend val -= 1. just use val-- – rolfl Jan 7 '14 at 17:44
2  
@Jaime - I only open my mouth to swap feet. Appreciate that you didn't add 'idiot' to your comment ;-) – rolfl Jan 7 '14 at 19:03
up vote 0 down vote

Consider stripping the method down to the essential requests.post() call, and using the @retry decorator instead.

share|improve this answer
    
Thank you, I will try the retrying package. Note that I reverted your edit, as this question is about the general case of do-while in Python, not the specific HTTP request issue that is used as an example of why it is necessary. – dotancohen Apr 6 '15 at 19:55
    
But a do-while differs from a while in exactly that aspect: A thing is done at least once and maybe again and again. There is at least a strong relation to the idea of a retry in that. – Alfe Apr 6 '15 at 21:00

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.