Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.python > #74254 > unrolled thread

Help me write better Code

Back to article view | Back to comp.lang.python

Contents

  Help me write better Code sssdevelop <sssdevelop@gmail.com> - 2014-07-09 07:27 -0700
    Re: Help me write better Code Mark Lawrence <breamoreboy@yahoo.co.uk> - 2014-07-09 16:44 +0100
      Re: Help me write better Code sssdevelop <sssdevelop@gmail.com> - 2014-07-10 07:39 -0700
        Re: Help me write better Code Mark Lawrence <breamoreboy@yahoo.co.uk> - 2014-07-10 15:51 +0100
      Re: Help me write better Code Rustom Mody <rustompmody@gmail.com> - 2014-07-10 11:38 -0700
    Re: Help me write better Code Ian Kelly <ian.g.kelly@gmail.com> - 2014-07-09 12:16 -0600
      Re: Help me write better Code sssdevelop <sssdevelop@gmail.com> - 2014-07-10 07:38 -0700
    Re: Help me write better Code Terry Reedy <tjreedy@udel.edu> - 2014-07-09 14:46 -0400
      Re: Help me write better Code sssdevelop <sssdevelop@gmail.com> - 2014-07-10 07:38 -0700

#74254 — Help me write better Code

Hello,

I have working code - but looking for better/improved code. Better coding practices, better algorithm :) 

Problem: Given sequence of increasing integers, print blocks of consecutive integers. 

Example: 

Input: [10, 11, 12, 15]
Output: [10, 11, 12]

Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
Outout: [67, 68], [91, 92, 93, 94]
 
My code looks as below: 
-----------------------------
#!/usr/bin/python
a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
#a = []
#a = [10]
#a = [10, 11, 12, 15]
print "Input: "
print  a

prev = 0
blocks = []
tmp = []
last = 0
for element in a:
   if prev == 0:
      prev = element
      next
   if element == prev + 1:
       if tmp:
           pass
       else:
           tmp.append(prev)
       tmp.append(element)
   else:
       if tmp:
          blocks.append(tmp)
       tmp = []

   prev = element

if tmp:
    blocks.append(tmp)

if blocks:
    #print "I have repeated elements and those are:"
    for b in blocks:
        print b

-----------------------

thank you in advance!

[toc] | [next] | [standalone]

#74260

On 09/07/2014 15:27, sssdevelop wrote:
> Hello,
>
> I have working code - but looking for better/improved code. Better coding practices, better algorithm :)
>
> Problem: Given sequence of increasing integers, print blocks of consecutive integers.
>
> Example:
>
> Input: [10, 11, 12, 15]
> Output: [10, 11, 12]
>
> Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> Outout: [67, 68], [91, 92, 93, 94]
>
> My code looks as below:
> -----------------------------
> #!/usr/bin/python
> a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> #a = []
> #a = [10]
> #a = [10, 11, 12, 15]
> print "Input: "
> print  a
>
> prev = 0
> blocks = []
> tmp = []
> last = 0
> for element in a:
>     if prev == 0:
>        prev = element
>        next
>     if element == prev + 1:
>         if tmp:
>             pass
>         else:
>             tmp.append(prev)
>         tmp.append(element)
>     else:
>         if tmp:
>            blocks.append(tmp)
>         tmp = []
>
>     prev = element
>
> if tmp:
>      blocks.append(tmp)
>
> if blocks:
>      #print "I have repeated elements and those are:"
>      for b in blocks:
>          print b
>
> -----------------------
>
> thank you in advance!
>

Adopted from here https://docs.python.org/3.0/library/itertools.html

data = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
for k, g in groupby(enumerate(data), lambda t:t[0]-t[1]):
     group = list(map(operator.itemgetter(1), g))
     if len(group) > 1:
         print(group)

 >>>
[67, 68]
[91, 92, 93, 94]
 >>>

-- 
My fellow Pythonistas, ask not what our language can do for you, ask 
what you can do for our language.

Mark Lawrence

---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com

[toc] | [prev] | [next] | [standalone]

#74309

Mark - thank you so much. You have suggested be new best tool/module. 
It's going to help me many places. Was not aware of such powerful tool. 

thank you,



On Wednesday, July 9, 2014 9:14:01 PM UTC+5:30, Mark Lawrence wrote:
> On 09/07/2014 15:27, sssdevelop wrote:
> 
> > Hello,
> 
> >
> 
> > I have working code - but looking for better/improved code. Better coding practices, better algorithm :)
> 
> >
> 
> > Problem: Given sequence of increasing integers, print blocks of consecutive integers.
> 
> >
> 
> > Example:
> 
> >
> 
> > Input: [10, 11, 12, 15]
> 
> > Output: [10, 11, 12]
> 
> >
> 
> > Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> 
> > Outout: [67, 68], [91, 92, 93, 94]
> 
> >
> 
> > My code looks as below:
> 
> > -----------------------------
> 
> > #!/usr/bin/python
> 
> > a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> 
> > #a = []
> 
> > #a = [10]
> 
> > #a = [10, 11, 12, 15]
> 
> > print "Input: "
> 
> > print  a
> 
> >
> 
> > prev = 0
> 
> > blocks = []
> 
> > tmp = []
> 
> > last = 0
> 
> > for element in a:
> 
> >     if prev == 0:
> 
> >        prev = element
> 
> >        next
> 
> >     if element == prev + 1:
> 
> >         if tmp:
> 
> >             pass
> 
> >         else:
> 
> >             tmp.append(prev)
> 
> >         tmp.append(element)
> 
> >     else:
> 
> >         if tmp:
> 
> >            blocks.append(tmp)
> 
> >         tmp = []
> 
> >
> 
> >     prev = element
> 
> >
> 
> > if tmp:
> 
> >      blocks.append(tmp)
> 
> >
> 
> > if blocks:
> 
> >      #print "I have repeated elements and those are:"
> 
> >      for b in blocks:
> 
> >          print b
> 
> >
> 
> > -----------------------
> 
> >
> 
> > thank you in advance!
> 
> >
> 
> 
> 
> Adopted from here https://docs.python.org/3.0/library/itertools.html
> 
> 
> 
> data = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> 
> for k, g in groupby(enumerate(data), lambda t:t[0]-t[1]):
> 
>      group = list(map(operator.itemgetter(1), g))
> 
>      if len(group) > 1:
> 
>          print(group)
> 
> 
> 
>  >>>
> 
> [67, 68]
> 
> [91, 92, 93, 94]
> 
>  >>>
> 
> 
> 
> -- 
> 
> My fellow Pythonistas, ask not what our language can do for you, ask 
> 
> what you can do for our language.
> 
> 
> 
> Mark Lawrence
> 
> 
> 
> ---
> 
> This email is free from viruses and malware because avast! Antivirus protection is active.
> 
> http://www.avast.com

[toc] | [prev] | [next] | [standalone]

#74310

On 10/07/2014 15:39, sssdevelop wrote:
>
> Mark - thank you so much. You have suggested be new best tool/module.
> It's going to help me many places. Was not aware of such powerful tool.
>
> thank you,
>

I'm pleased to see that you have several answers.  In return would you 
please use the mailing list 
https://mail.python.org/mailman/listinfo/python-list or read and action 
this https://wiki.python.org/moin/GoogleGroupsPython to prevent us 
seeing double line spacing and single line paragraphs, thanks.

Would you also not top post.  In other words, place your replies at the 
bottom or interspersed between the paragraphs that you're replying to, 
thanks again.

-- 
My fellow Pythonistas, ask not what our language can do for you, ask 
what you can do for our language.

Mark Lawrence

---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com

[toc] | [prev] | [next] | [standalone]

#74329

On Wednesday, July 9, 2014 9:14:01 PM UTC+5:30, Mark Lawrence wrote:
> On 09/07/2014 15:27, sssdevelop wrote:
> > Hello,
> > I have working code - but looking for better/improved code. Better coding practices, better algorithm :)
> > Problem: Given sequence of increasing integers, print blocks of consecutive integers.
> > Example:
> > Input: [10, 11, 12, 15]
> > Output: [10, 11, 12]
> > Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> > Outout: [67, 68], [91, 92, 93, 94]
> > My code looks as below:
> > -----------------------------
> > #!/usr/bin/python
> > a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> > #a = []
> > #a = [10]
> > #a = [10, 11, 12, 15]
> > print "Input: "
> > print  a
> > prev = 0
> > blocks = []
> > tmp = []
> > last = 0
> > for element in a:
> >     if prev == 0:
> >        prev = element
> >        next
> >     if element == prev + 1:
> >         if tmp:
> >             pass
> >         else:
> >             tmp.append(prev)
> >         tmp.append(element)
> >     else:
> >         if tmp:
> >            blocks.append(tmp)
> >         tmp = []
> >     prev = element
> > if tmp:
> >      blocks.append(tmp)
> > if blocks:
> >      #print "I have repeated elements and those are:"
> >      for b in blocks:
> >          print b
> > -----------------------
> > thank you in advance!

> Adopted from here https://docs.python.org/3.0/library/itertools.html

> data = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> for k, g in groupby(enumerate(data), lambda t:t[0]-t[1]):
>      group = list(map(operator.itemgetter(1), g))
>      if len(group) > 1:
>          print(group)

> [67, 68]
> [91, 92, 93, 94]

> -- 


Mark's version without the print.

Which says in a different way, Terry's point-2 :

> 2. Separate interface code that gets input and presents output from the
> function that processes the increasing sequence. The function should not
> care whether the ints come from a user, file, or calculation.


$ python3
Python 3.4.1 (default, Jun  9 2014, 10:28:44) 
[GCC 4.8.3] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from operator import itemgetter
>>> from itertools import groupby
>>> data = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99] 
>>> [group  for k,g in groupby(enumerate(data), lambda t:t[0]-t[1])
...         for group in [list(map(itemgetter(1), g))]
...         if len(group) > 1
... 
... ]
[[67, 68], [91, 92, 93, 94]]
>>> 

You can replace the outermost '[]' with '()'
 

[toc] | [prev] | [next] | [standalone]

#74270

On Wed, Jul 9, 2014 at 8:27 AM, sssdevelop <sssdevelop@gmail.com> wrote:
> prev = 0
> blocks = []
> tmp = []
> last = 0
> for element in a:
>    if prev == 0:

Is 0 allowed to be in the input list? What would happen if it were?

>       next

This line doesn't do anything.  It looks up the builtin function named
next and then does nothing with it.  I suspect you meant to use the
keyword 'continue' here.

>        if tmp:
>            pass
>        else:
>            tmp.append(prev)

        if not tmp:
            tmp.append(prev)

Also, give tmp a more meaningful name. Call it "current_block" or
something descriptive like that.

[toc] | [prev] | [next] | [standalone]

#74308

thank you so much!


On Wednesday, July 9, 2014 11:46:41 PM UTC+5:30, Ian wrote:
> On Wed, Jul 9, 2014 at 8:27 AM, sssdevelop <sssdevelop@gmail.com> wrote:
> 
> > prev = 0
> 
> > blocks = []
> 
> > tmp = []
> 
> > last = 0
> 
> > for element in a:
> 
> >    if prev == 0:
> 
> 
> 
> Is 0 allowed to be in the input list? What would happen if it were?
> 
> 
> 
> >       next
> 
> 
> 
> This line doesn't do anything.  It looks up the builtin function named
> 
> next and then does nothing with it.  I suspect you meant to use the
> 
> keyword 'continue' here.
> 
> 
> 
> >        if tmp:
> 
> >            pass
> 
> >        else:
> 
> >            tmp.append(prev)
> 
> 
> 
>         if not tmp:
> 
>             tmp.append(prev)
> 
> 
> 
> Also, give tmp a more meaningful name. Call it "current_block" or
> 
> something descriptive like that.

[toc] | [prev] | [next] | [standalone]

#74272

On 7/9/2014 10:27 AM, sssdevelop wrote:
> Hello,
>
> I have working code - but looking for better/improved code. Better coding practices, better algorithm :)
>
> Problem: Given sequence of increasing integers, print blocks of consecutive integers.

> Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> Outout: [67, 68], [91, 92, 93, 94]

Recommendations:
1. If you are just beginning Python, use the current version, now 3.4.

2. Separate interface code that gets input and presents output from the 
function that processes the increasing sequence. The function should not 
care whether the ints come from a user, file, or calculation.

3. Think in terms of iterables and iterators rather than lists (this is 
clearer in 3.x, where some builtins have been converted). The function 
should not care what class is used to convey the sequence of numbers. 
This happens to make it easier to solve the 'pump-priming' problem you 
stumbled over.

4. For designing a loop, what is the loop invariant that you want, that 
will make writing the code easy. For this problem, "tem is a non-emptly 
list of consecutive integers". Remember that a list of one int 
qualifies. Using for loops with the proper iterable solves the other 
part of the loop invariant: the current item is the next item to be 
compared to the last item of tem. If tem is always non-empty, that 
comparison is always possible.

5. Remember that Python code is generic unless constrained. What should 
happen if the function gets non-int numbers, with or without an integer 
value? What should happen if the sequence is not increasing, but 
contains consecutive subsequences. For beginning code, one could decide 
to meet the spec given for input that meets the condition and not care 
otherwise.  The code below works for any sequence (even infinite) of 
objects that can be incremented by 1 and compared to the next.

6. Write an automated test. For one test, something like this works.

ci = consec([51, 53, 55, 67, 68, 91, 92, 93, 94, 99])
print(next(ci) == [67, 68], next(ci) == [91, 92, 93, 94])

but since you (properly) noted several test cases

a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
#a = []
#a = [10]
#a = [10, 11, 12, 15]

I went ahead and used unittest, at the cost of three lines of 
'boilerplate' code. I added a case with a final consecutive sequence. 
Good thing, because it initially failed because I initially forgot to 
check tem after the loop.

import unittest

def consec(iterable):
     "Yield blocks of consecutive integers as a list."
     it = iter(iterable)
     first = next(it)
     tem = [first]
     for n in it:
         # tem is a non-empty list of consecutive ints
         if n == tem[-1] + 1:
             tem.append(n)
         else:
             if len(tem) >= 2:
                 yield tem
             tem = [n]
     if len(tem) >= 2:
         yield tem

class Test(unittest.TestCase):
     def test_consec(self):
         def eq(seq, blocks):
             self.assertEqual(list(consec(seq)), blocks)
         eq((), [])
         eq([1], [])
         eq([1,2,3], [[1,2,3]])  # block at beginning or end
         eq([-1, 1,2,3, 5], [[1,2,3]])  # block in middle
         eq((-1, 1,2,3, 5, 7,8,9, 11), [[1,2,3], [7,8,9]])  # 2 blocks

unittest.main(verbosity=2)
 >>>
test_consec (__main__.Test) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.016s

OK

-- 
Terry Jan Reedy

[toc] | [prev] | [next] | [standalone]

#74307

Thank you so much Terry Jan Reedy. You have given best advice - yup, i am beginner in Python. 
Your reply has done grooming :) 

thx,



On Thursday, July 10, 2014 12:16:48 AM UTC+5:30, Terry Reedy wrote:
> On 7/9/2014 10:27 AM, sssdevelop wrote:
> 
> > Hello,
> 
> >
> 
> > I have working code - but looking for better/improved code. Better coding practices, better algorithm :)
> 
> >
> 
> > Problem: Given sequence of increasing integers, print blocks of consecutive integers.
> 
> 
> 
> > Input: [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> 
> > Outout: [67, 68], [91, 92, 93, 94]
> 
> 
> 
> Recommendations:
> 
> 1. If you are just beginning Python, use the current version, now 3.4.
> 
> 
> 
> 2. Separate interface code that gets input and presents output from the 
> 
> function that processes the increasing sequence. The function should not 
> 
> care whether the ints come from a user, file, or calculation.
> 
> 
> 
> 3. Think in terms of iterables and iterators rather than lists (this is 
> 
> clearer in 3.x, where some builtins have been converted). The function 
> 
> should not care what class is used to convey the sequence of numbers. 
> 
> This happens to make it easier to solve the 'pump-priming' problem you 
> 
> stumbled over.
> 
> 
> 
> 4. For designing a loop, what is the loop invariant that you want, that 
> 
> will make writing the code easy. For this problem, "tem is a non-emptly 
> 
> list of consecutive integers". Remember that a list of one int 
> 
> qualifies. Using for loops with the proper iterable solves the other 
> 
> part of the loop invariant: the current item is the next item to be 
> 
> compared to the last item of tem. If tem is always non-empty, that 
> 
> comparison is always possible.
> 
> 
> 
> 5. Remember that Python code is generic unless constrained. What should 
> 
> happen if the function gets non-int numbers, with or without an integer 
> 
> value? What should happen if the sequence is not increasing, but 
> 
> contains consecutive subsequences. For beginning code, one could decide 
> 
> to meet the spec given for input that meets the condition and not care 
> 
> otherwise.  The code below works for any sequence (even infinite) of 
> 
> objects that can be incremented by 1 and compared to the next.
> 
> 
> 
> 6. Write an automated test. For one test, something like this works.
> 
> 
> 
> ci = consec([51, 53, 55, 67, 68, 91, 92, 93, 94, 99])
> 
> print(next(ci) == [67, 68], next(ci) == [91, 92, 93, 94])
> 
> 
> 
> but since you (properly) noted several test cases
> 
> 
> 
> a = [51, 53, 55, 67, 68, 91, 92, 93, 94, 99]
> 
> #a = []
> 
> #a = [10]
> 
> #a = [10, 11, 12, 15]
> 
> 
> 
> I went ahead and used unittest, at the cost of three lines of 
> 
> 'boilerplate' code. I added a case with a final consecutive sequence. 
> 
> Good thing, because it initially failed because I initially forgot to 
> 
> check tem after the loop.
> 
> 
> 
> import unittest
> 
> 
> 
> def consec(iterable):
> 
>      "Yield blocks of consecutive integers as a list."
> 
>      it = iter(iterable)
> 
>      first = next(it)
> 
>      tem = [first]
> 
>      for n in it:
> 
>          # tem is a non-empty list of consecutive ints
> 
>          if n == tem[-1] + 1:
> 
>              tem.append(n)
> 
>          else:
> 
>              if len(tem) >= 2:
> 
>                  yield tem
> 
>              tem = [n]
> 
>      if len(tem) >= 2:
> 
>          yield tem
> 
> 
> 
> class Test(unittest.TestCase):
> 
>      def test_consec(self):
> 
>          def eq(seq, blocks):
> 
>              self.assertEqual(list(consec(seq)), blocks)
> 
>          eq((), [])
> 
>          eq([1], [])
> 
>          eq([1,2,3], [[1,2,3]])  # block at beginning or end
> 
>          eq([-1, 1,2,3, 5], [[1,2,3]])  # block in middle
> 
>          eq((-1, 1,2,3, 5, 7,8,9, 11), [[1,2,3], [7,8,9]])  # 2 blocks
> 
> 
> 
> unittest.main(verbosity=2)
> 
>  >>>
> 
> test_consec (__main__.Test) ... ok
> 
> 
> 
> ----------------------------------------------------------------------
> 
> Ran 1 test in 0.016s
> 
> 
> 
> OK
> 
> 
> 
> -- 
> 
> Terry Jan Reedy

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.python

csiph-web