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

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

suggestions for improving code fragment please

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

Contents

  suggestions for improving code fragment please The Night Tripper <jkn+gg@nicorp.co.uk> - 2013-02-28 19:47 +0000
    Re: suggestions for improving code fragment please Rick Johnson <rantingrickjohnson@gmail.com> - 2013-02-28 11:55 -0800
    Re: suggestions for improving code fragment please Joel Goldstick <joel.goldstick@gmail.com> - 2013-02-28 14:56 -0500
      Re: suggestions for improving code fragment please The Night Tripper <jkn+gg@nicorp.co.uk> - 2013-02-28 23:20 +0000
    Re: suggestions for improving code fragment please Ian Kelly <ian.g.kelly@gmail.com> - 2013-02-28 12:58 -0700
    Re: suggestions for improving code fragment please Tim Chase <python.list@tim.thechases.com> - 2013-02-28 14:37 -0600
    Re: suggestions for improving code fragment please MRAB <python@mrabarnett.plus.com> - 2013-02-28 21:13 +0000
    Re: suggestions for improving code fragment please Mitya Sirenef <msirenef@lightbird.net> - 2013-02-28 16:22 -0500
    Re: suggestions for improving code fragment please Dave Angel <davea@davea.name> - 2013-02-28 16:28 -0500
    Re: suggestions for improving code fragment please Terry Reedy <tjreedy@udel.edu> - 2013-02-28 16:30 -0500
    Re: suggestions for improving code fragment please Tim Chase <python.list@tim.thechases.com> - 2013-02-28 16:44 -0600
    Re: suggestions for improving code fragment please Steven D'Aprano <steve+comp.lang.python@pearwood.info> - 2013-03-01 02:44 +0000

#40163 — suggestions for improving code fragment please

Hi there
    I'm being very dumb ... how can I simplify this fragment?


        if arglist:
            arglist.pop(0)
            if arglist:
                self.myparm1 = arglist.pop(0)
                if arglist:
                    self.myparm2 = arglist.pop(0)
                    if arglist:
                        self.myparm3 = arglist.pop(0)
                        if arglist:
                            self.parm4 = arglist.pop(0)
        # ...

    Thanks
    J^n

[toc] | [next] | [standalone]

#40165

On Thursday, February 28, 2013 1:47:12 PM UTC-6, The Night Tripper wrote:
>     I'm being very dumb ... how can I simplify this fragment?
> 
>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)

Depends. If the length of arglist is "known" you could simply unpack it:

  a,b,c,d = (1,2,3,4)    

If the length is unknown, a while loop would do the trick. All you need is to figure out what "truth condition" to test for on each iteration of the while loop.

 while truthCondition:
    #assign variable to value

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

#40166

[Multipart message — attachments visible in raw view] — view raw

On Thu, Feb 28, 2013 at 2:47 PM, The Night Tripper <jkn+gg@nicorp.co.uk>wrote:

> Hi there
>     I'm being very dumb ... how can I simplify this fragment?
>
i = 0
while arglist:
    self.myparm[i] = arglist.pop(0)
    i += 1



>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)
>         # ...
>
>     Thanks
>     J^n
>
>
> --
> http://mail.python.org/mailman/listinfo/python-list
>



-- 
Joel Goldstick
http://joelgoldstick.com

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

#40200

Hi All
    thanks very much for the various suggestions - very helpful.

I think I like one of the 'just catch the exception' approaches,
or using Mitya's helper function, which I was clutching towards myself.

Either way, lots of food for thought. This forum really is one of the best 
places around...

    Cheers
    Jon N
 

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

#40167

On Thu, Feb 28, 2013 at 12:47 PM, The Night Tripper <jkn+gg@nicorp.co.uk> wrote:
> Hi there
>     I'm being very dumb ... how can I simplify this fragment?
>
>
>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)

Perhaps this would work for you:

if arglist:
    defaults = [self.parm1, self.parm2, self.parm3, self.parm4]
    arglist = arglist[1:] + defaults[len(arglist)-1:]
    self.parm1, self.parm2, self.parm3, self.parm4 = arglist[:4]

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

#40171

On 2013-02-28 19:47, The Night Tripper wrote:
> Hi there
>     I'm being very dumb ... how can I simplify this fragment?
> 
> 
>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)

If they're arbitrarily named attributes of the "self", you could do
something like

  for attr in ("myparm1", "myparm2", "myparm3", ...):
    if arglist:
      setattr(self, attr, arglist.pop(0))
    else:
      break

-tkc

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

#40181

On 2013-02-28 19:47, The Night Tripper wrote:
> Hi there
>      I'm being very dumb ... how can I simplify this fragment?
>
>
>          if arglist:
>              arglist.pop(0)
>              if arglist:
>                  self.myparm1 = arglist.pop(0)
>                  if arglist:
>                      self.myparm2 = arglist.pop(0)
>                      if arglist:
>                          self.myparm3 = arglist.pop(0)
>                          if arglist:
>                              self.parm4 = arglist.pop(0)
>          # ...
>
You could just catch the exception:

     try:
         arglist.pop(0)
         self.myparm1 = arglist.pop(0)
         self.myparm2 = arglist.pop(0)
         self.myparm3 = arglist.pop(0)
         self.parm4 = arglist.pop(0)
     except IndexError:
         pass

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

#40185

On 02/28/2013 02:47 PM, The Night Tripper wrote:
> Hi there
 > I'm being very dumb ... how can I simplify this fragment?
 >
 >
 > if arglist:
 > arglist.pop(0)
 > if arglist:
 > self.myparm1 = arglist.pop(0)
 > if arglist:
 > self.myparm2 = arglist.pop(0)
 > if arglist:
 > self.myparm3 = arglist.pop(0)
 > if arglist:
 > self.parm4 = arglist.pop(0)
 > # ...
 >
 > Thanks
 > J^n
 >
 >


I often use this convenience function:

def getitem(seq, index, default=None):
"""Get item from an `seq` at `index`, return default if index out of 
range."""
try : return seq[index]
except IndexError : return default


If you're ok with setting myparm values to default None, you can do:

self.myparm1, self.myparm2, self.myparm3, self.myparm4 = \
(getitem(arglist, n) for n in range(4))


If you only want to set them when they are in arglist:

for n in range(4):
val = getitem(arglist, n)
if val is not None:
setattr(self, "myparm%d" % (n+1), val)

-m



-- 
Lark's Tongue Guide to Python: http://lightbird.net/larks/

“So many books, so little time.”
― Frank Zappa

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

#40187

On 02/28/2013 03:37 PM, Tim Chase wrote:
> On 2013-02-28 19:47, The Night Tripper wrote:
>> Hi there
>>      I'm being very dumb ... how can I simplify this fragment?
>>
>>
>>          if arglist:
>>              arglist.pop(0)
>>              if arglist:
>>                  self.myparm1 = arglist.pop(0)
>>                  if arglist:
>>                      self.myparm2 = arglist.pop(0)
>>                      if arglist:
>>                          self.myparm3 = arglist.pop(0)
>>                          if arglist:
>>                              self.parm4 = arglist.pop(0)
>
> If they're arbitrarily named attributes of the "self", you could do
> something like
>
>    for attr in ("myparm1", "myparm2", "myparm3", ...):
>      if arglist:
>        setattr(self, attr, arglist.pop(0))
>      else:
>        break
>
> -tkc
>
>
>
Or something like (untested):

     for name, value in zip(["myparm1", "myparm2", "myparm3"], arglist):
          setattr(self, name, value)
     arglist = []     #if you care how it ends up


-- 
DaveA

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

#40188

On 2/28/2013 2:47 PM, The Night Tripper wrote:
> Hi there
>      I'm being very dumb ... how can I simplify this fragment?
>
>
>          if arglist:
>              arglist.pop(0)
>              if arglist:
>                  self.myparm1 = arglist.pop(0)
>                  if arglist:
>                      self.myparm2 = arglist.pop(0)
>                      if arglist:
>                          self.myparm3 = arglist.pop(0)
>                          if arglist:
>                              self.parm4 = arglist.pop(0)

To literally do the same thing

try:
     arglist.pop(0)
     self.myparm1 = arglist.pop(0)
     self.myparm2 = arglist.pop(0)
     self.myparm3 = arglist.pop(0)
     self.parm4 = arglist.pop(0)
except IndexError:
     pass

However, repeated popping from the front is O(n**2) instead of O(n).
Following should do the same, including removal from original arglist.

it = iter(arglist)
n = 0
try:
     next(it); n += 1
     self.myparm1 = next(it); n += 1
     self.myparm2 = next(it); n += 1
     self.myparm3 = next(it); n += 1
     self.parm4 = next(it); n += 1
except StopIteration:
     pass
arglist = arglist[n:]

-- 
Terry Jan Reedy

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

#40198

On 2013-02-28 16:28, Dave Angel wrote:
> On 02/28/2013 03:37 PM, Tim Chase wrote:
> >    for attr in ("myparm1", "myparm2", "myparm3", ...):
> >      if arglist:
> >        setattr(self, attr, arglist.pop(0))
> >      else:
> >        break
> >
> Or something like (untested):
> 
>      for name, value in zip(["myparm1", "myparm2", "myparm3"],
> arglist): setattr(self, name, value)
>      arglist = []     #if you care how it ends up

The OP's code modified arglist by .pop(0) so I maintained the same
behavior.  This is useful if additional arguments beyond the N named
ones are used for some other purpose and you don't want to figure out
how many were taken. Otherwise, if one wants to keep arglist, Dave's
zip() solution is a cleaner way to go.

-tkc

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

#40214

On Thu, 28 Feb 2013 19:47:12 +0000, The Night Tripper wrote:

> Hi there
>     I'm being very dumb ... how can I simplify this fragment?

I suggest that the best way to simplify that fragment is to change the 
design of your class so it isn't so horrible. As it stands now, your 
class defines an arbitrary number of params, which *may or may not exist*:

>         if arglist:
>             arglist.pop(0)
>             if arglist:
>                 self.myparm1 = arglist.pop(0)
>                 if arglist:
>                     self.myparm2 = arglist.pop(0)
>                     if arglist:
>                         self.myparm3 = arglist.pop(0)
>                         if arglist:
>                             self.parm4 = arglist.pop(0)
>         # ...

So using your class is a horrible experience:

if hasattr(instance, 'param1'):
    do_something_with(instance.param1)
else:
    fall_back_when_param1_doesnt_exist()
if hasattr(instance, 'param2'):
    do_something_with(instance.param2)
else:
    fall_back_when_param2_doesnt_exist()
if hasattr(instance, 'param3'):
    print "Do you hate this class yet?"



We have a perfectly good programming idiom to deal with a variable number 
of values: the list, or tuple if you prefer. So here's an alternative:

self.params = arglist[1:]

If order is not important:

self.params = set(arglist[1:])


If you have to map names to values:

self.params = dict(
    ('param%d' % i, value) for i, value in enumerate(arglist[1:])
    )


If you absolutely must have named attributes, I recommend that you choose 
a default value that indicates "missing". Conventionally, that is None, 
but you can always create your own sentinel value if needed:

SENTINEL = object()
arglist = arglist + [SENTINEL]*20
for i in range(1, 21):
    setattr(self, 'param%d' % i, arglist[i])



-- 
Steven

[toc] | [prev] | [standalone]

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

csiph-web