Improving efficiency for finding longest contiguous non-decreasing substring

I was working on a problem set and I came across this problem:

Assume s is a string of lower case characters.

Write a program that prints the longest substring of s in which the letters occur in alphabetical order. For example, if s = 'azcbobobegghakl', then your program should print:
Longest substring in alphabetical order is: beggh


In the case of ties, print the first substring. For example, if s = 'abcbcd', then your program should print
Longest substring in alphabetical order is: abc


I actually was able to get a solution, but it was the most inefficient ever! And here's what I came up with:

def test():
    index = 1
    prev_index = 0
    count = 0
    global largest
    largest = ''
    test = s[prev_index]

    while count  ord(s[prev_index]):
            test += s[index]
            index += 1
            prev_index += 1
        elif ord(s[index]) == ord(s[prev_index]):
            test += s[index]
            index += 1
            prev_index += 1
        else:
            if len(largest) < len(test):
                largest = test[:]
            test = s[index]
            prev_index += 1
            index += 1
        count += 1
    return largest
try:
     test()
except IndexError:
     pass
finally:
     print largest

Can anyone give me an example of better code that produces the same output?

Your algorithm is basically fine, but it is expressed in the code clumsily.

The biggest problem is the interface of the function: it should accept a string parameter and return a string. Instead, your function takes s from its environment and uses the global variable largest to store the answer. Although there is a return largest statement, it is very misleading. What actually happens is that s[index] raises an IndexError, so that the return largest is never reached, and the answer is passed back using the global variable instead.

The root cause for the IndexError is that there are too many variables, causing confusion. With each iteration through the loop, you increment index, prev_index, and count. That means that those three variables could be reduced to one, which could just be called i. (If you think about it, you'll find that count is always one less than index. Since the loop condition is count = for the test.

Python strings are immutable, so largest = test[:] could just be largest = test.

Putting everything together…

def longest_nondecreasing_substring(s):
    if s is None or len(s) = ord(s[i - 1]):
            test += s[i]
        else:
            if len(longest) < len(test):
                longest = test
            test = s[i]
    if len(longest) < len(test):
        longest = test
    return longest

StackExchange Code Review Q#33291, answer score: 2