Grouping consecutive numbers into ranges in Python 3.2 : New version for set

Question

def range_find(pageset,upperlimit=1000):

    pagerangelist=[]

    for page in range(0,upperlimit):

        if page in pageset and previouspage in pageset:pagerangelist[-1].append(page)
        elif page in pageset and not (previouspage in pageset):pagerangelist.append([page])
        else: pass

        previouspage=page

    pagerangestringlist=[]

    for pagerange in pagerangelist:

        pagerangestringlist.append(str(pagerange[0])+('-'+str(pagerange[-1]))*((len(pagerange)-1)>0))


    return(','.join(pagerangestringlist))


print(range_find({1,2,5,7,8,11,19,25,29,30,31,39,40,41,42,43,44,56,57,59,60,70,71,72,100,101,103,104,105}))

Answer 1

def range_find(pageset,upperlimit=1000):

    pagerangelist=[]

    for page in range(0,upperlimit):

There is no reason to include the 0. Why don't you calculate the upperlimit by max(pageset)?

        if page in pageset and previouspage in pageset:pagerangelist[-1].append(page)
        elif page in pageset and not (previouspage in pageset):pagerangelist.append([page])
        else: pass

Don't squish all that stuff onto one line. It makes it harder to read. Having an else: pass case is useless

        previouspage=page

Since you are counting, just use page - 1 instead of previouspage

    pagerangestringlist=[]

    for pagerange in pagerangelist:

        pagerangestringlist.append(str(pagerange[0])+('-'+str(pagerange[-1]))*((len(pagerange)-1)>0))

Too clever. Just use an if. This technique makes it harder to see what is going on. Also pointless extra parens.

    return(','.join(pagerangestringlist))

Don't put parens around your return statement expressions.

Algorithmwise this may be problematic because it will slower for books with many pages even if you don't use many pages from them. For example

range_find({1, 2, 1000000},100000000)

Will spend a long time churning through pages that aren't really of interest.