statements in control structures (Re: Conditional Expressions don't solve the problem)

[Huaiyu Zhu]

On Thu, 18 Oct 2001 02:10:40 -0600, Andrew Dalke <dalke at dalkescientific.com>
wrote: 
>Yep.  My belief is that cases where your proposal shines doesn't
>occur very often in real code, and that the additional flexibility
>will instead lead to more obsfucated code.
[...]

>It's easy.  Scan the standard library and find a chunk of code which
>you believe will be improved with your construct.

OK, I'll bite the bullet.  ;-)

A search through the source distribution found 1004 "break"s, most of them
(about 800) are break-statements:

$ rgrep -R "*.py" break . |grep -v breaks| grep -v cbreak|wc
    927    3357   45879

I picked a few files that have more than a few break statements, and
searched for break.  Almost all of them are in this style:

while 1:
    x = something
    if not x: break
    ...

Following are a few examples of while-loops.  I don't think I'll spend more
time on this.  Anybody can do their own search if they want real life
examples.  Also I'm not sure how to search for possible places where nested
if-else ccould be changed to flat elif.  The number of pontential instances
to look for is too large:

rgrep -R "*.py" else . |wc
   3098    7655  126721


Doing these examples, I got a clearer view on why the change would be good:
The new syntax allows one to say
        " do ... under this condition ... "
instead of the more twisted logic of 
        " do ..., but if not this, change ... "
It also displays the loop condition prominently, instead of displaying a
dummy "while 1" and bury the substance inside.

Here are 6 examples [Disclaimer: It's about half an hour quick hack
including the searching.  Almost mechanical change.  No guarantee I really
understand the original code before changing them. No checking, debugging or
whatsoever, etc.]


./Demo/pdist/makechangelog.py:
--------------------------------------------(1)

    while 1:
        file = getnextfile(f)
        if not file: break
        revs = []
        while 1:
            rev = getnextrev(f, file)
            if not rev:
                break
            revs.append(rev)
        if revs:
            allrevs[len(allrevs):] = revs

Change to:

    while file = getnextfile(f); file:
        revs = []
        while rev = getnextrev(f, file); rev:
            revs.append(rev)
        if revs:
            allrevs[len(allrevs):] = revs

--------------------------------------------(2) 
    while 1:
        line = f.readline()
        if not line: return None
        if startprog.match(line) >= 0:
            file = startprog.group(1)
            while 1:
                line = f.readline()
                if not line: return None
                if line[:10] == '='*10: return None
                if line[:10] == '-'*10: break
            return file


changed to
    while line = f.readline(); line:
        if startprog.match(line) >= 0:
            file = startprog.group(1)
            while line = f.readline(); line[:10] != '-'*10:
                if not line: return None
                if line[:10] == '='*10: return None
            return file
    return None


Lib/ftplib.py:
--------------------------------------------(3)
    def getmultiline(self):
        line = self.getline()
        if line[3:4] == '-':
            code = line[:3]
            while 1:
                nextline = self.getline()
                line = line + ('\n' + nextline)
                if nextline[:3] == code and \
                        nextline[3:4] != '-':
                    break
        return line

changed to

    def getmultiline(self):
        code = None
        lines = []
        while line = self.getline(); lines.append(line); line[3:4] == '-':
            if code != line[:3]: break
            else: code = line[:3]
        return '\n'.join(lines)


--------------------------------------------(4)
        while 1:
            data = conn.recv(blocksize)
            if not data:
                break
            callback(data)

changed to:

        while data = conn.recv(blocksize); data:
            callback(data)

./Lib/mimify.py:  
--------------------------------------------(5)
    while 1:
        res = mime_code.search(line, pos)
        if res is None:
            break
        newline = newline + line[pos:res.start(0)] + \
                  chr(int(res.group(1), 16))
        pos = res.end(0)


changed

    while res = mime_code.search(line, pos); res is not None:
        newline = newline + line[pos:res.start(0)] + \
                  chr(int(res.group(1), 16))
        pos = res.end(0)

--------------------------------------------(6)

    # read header
    hfile = HeaderFile(ifile)
    while 1:
        line = hfile.readline()
        if not line:
            return
        if prefix and line[:len(prefix)] == prefix:
            line = line[len(prefix):]
            pref = prefix
        else:
            pref = ''
        line = mime_decode_header(line)
        if qp.match(line):
            quoted_printable = 1
            continue        # skip this header
        if decode_base64 and base64_re.match(line):
            is_base64 = 1
            continue
        ofile.write(pref + line)
        if not prefix and repl.match(line):
            # we're dealing with a reply message
            is_repl = 1
        mp_res = mp.match(line)
        if mp_res:
            multipart = '--' + mp_res.group(1)
        if he.match(line):
            break
    if is_repl and (quoted_printable or multipart):
        is_repl = 0

Changed to

    # read header
    hfile = HeaderFile(ifile)
    while line = hfile.readline(); line:
        if prefix and line.startswith(prefix):
            line = line[len(prefix):]
            pref = prefix
        else:
            pref = ''
        line = mime_decode_header(line)
        if qp.match(line):
            quoted_printable = 1
        elif decode_base64 and base64_re.match(line):
            is_base64 = 1
        else:
            ofile.write(pref + line)
            if not prefix and repl.match(line):
                is_repl = 1
            if mp_res = mp.match(line); mp_res:
                multipart = '--' + mp_res.group(1)
            if he.match(line):
                break
    else:
        return
    if is_repl and (quoted_printable or multipart):
        is_repl = 0