2

I have a list of class instances Child and I want to modify the toy attribute of each child using the list of toys returned from a function

There is a working solution below but I am wondering if there is a one-liner?

import random

class Child():
    def __init__(self, name, toy):
        self.name = name
        self.toy = toy

def change_toys(nChildren):
    toys = ['toy1', 'toy2', 'toy3', 'toy4']
    random.shuffle(toys)
    return toys[:nChildren]

child_list = [Child('Ngolo', None), Child('Kante', None)]

new_toys = change_toys(len(child_list))

for i in range(len(child_list)):
    child_list[i].toy = new_toys[i]
    print "%s has toy %s" %(child_list[i].name, child_list[i].toy)

Output (random toy assignement):

Ngolo has toy toy3
Kante has toy toy2

I tried:

[child.toy for child in child_list] = change_toys(nChildren)

But that doesn't work, I get

SyntaxError: can't assign to list comprehension

Any ideas?

juanpa.arrivillaga
  • 88,713
  • 10
  • 131
  • 172
  • 1
    If your working solution works, stick with that. Sometimes you still have to write `for` loops. – khelwood Apr 12 '17 at 08:53
  • 1
    You shouldn't use list comprehensions for side effects. Stick to the regular loop. Also note that you can `random.sample` to simplify `change_toys`. – jonrsharpe Apr 12 '17 at 08:54
  • If your solution works, you should rather post your question to http://codereview.stackexchange.com/ – Mike Scotty Apr 12 '17 at 08:56

2 Answers2

0

This won't be a one-liner, but you should write your loop in a cleaner, more pythonic way by avoiding the use of your index i:

for child, toy in zip(child_list, new_toys):
    child.toy = toy
    print "%s has toy %s" %(child.name, child.toy)
Thierry Lathuille
  • 23,663
  • 10
  • 44
  • 50
0

If you really want to use a list comprehension, you should make a new list creating new Child objects rather than mutating existing objects:

In [6]: new_list = [Child(c.name, toy) for c, toy in zip(child_list, change_toys(len(child_list)))]

In [7]: new_list
Out[7]: [<__main__.Child at 0x103ade208>, <__main__.Child at 0x103ade1d0>]

In [8]: c1, c2 = new_list

In [9]: c1.name, c1.toy, c2.name, c2.toy
Out[9]: ('Ngolo', 'toy4', 'Kante', 'toy2')

Compared to the original list:

In [10]: c1, c2 = child_list

In [11]: c1.name, c1.toy, c2.name, c2.toy
Out[11]: ('Ngolo', None, 'Kante', None)

If you'd rather stick with mutating your child instances (a reasonable approach) then your for-loop is just fine.

I would go with the for-loop using zip like @ThierryLathuille answer.

juanpa.arrivillaga
  • 88,713
  • 10
  • 131
  • 172