1
\$\begingroup\$
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}))
\$\endgroup\$
4
  • \$\begingroup\$ (a) Your indentation is wrtong. (b) You didn't reference the previous version of this question. (c) You didn't include the output from your test. (d) You didn't test the [1, 2, 4, 5, 3] case which requires merging two partitions. If you require the input to be sorted, please state that clearly or add sorted to assure it. \$\endgroup\$
    – S.Lott
    Commented Oct 7, 2011 at 15:22
  • \$\begingroup\$ @S.Lott, where is the indentation wrong? He's passing in a set so order doesn't matter. Your other points are good. \$\endgroup\$ Commented Oct 7, 2011 at 16:06
  • \$\begingroup\$ Since the indentation is now fixed, it's no longer wrong. \$\endgroup\$
    – S.Lott
    Commented Oct 7, 2011 at 16:17
  • \$\begingroup\$ @S.Lott, okay, so it was just that he failed to format the code correctly in his post, not that the indentation in the original code was incorrect. \$\endgroup\$ Commented Oct 7, 2011 at 17:13

1 Answer 1

2
\$\begingroup\$
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.

\$\endgroup\$

Your Answer

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

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