I'm generating a custom combination of sub lists without using itertools.

Here's what I've come up with:

input =  [[1, 2], [2, 3], [4, 3]]
output = [[1, 2, 2, 3], [1, 2, 4, 3], [2, 3, 4, 3]]

def getList():
    a = [[1, 2], [2, 3], [4, 3]]

c=[]                        # output list
l = len(getList())
for i in range(l):
    for j in range(i+1,l):

As the extend() updates the input list, I defined a function to redefine the input. Is it a recommended practice? What could be improved here?

  • 2
    \$\begingroup\$ Why don't you want to use itertools? Because using itertools is recommended practice... \$\endgroup\$ – Peilonrayz Nov 20 '17 at 9:52
  • \$\begingroup\$ @Peilonrayz no reason as such, was just trying how to achieve without it. \$\endgroup\$ – Van Peer Nov 20 '17 at 9:56
  • \$\begingroup\$ @VanPeer You have a hardcoded input() in get_List(), what would happen if the input changes? \$\endgroup\$ – Ludisposed Nov 20 '17 at 10:55
  • \$\begingroup\$ These are not all combinations of a list \$\endgroup\$ – Ludisposed Nov 20 '17 at 11:00
  • \$\begingroup\$ @Ludisposed thanks! assuming input doesn't change. i needed only the combinations specified in the output. i didn't mention all combinations, did i? anyways, updated. \$\endgroup\$ – Van Peer Nov 20 '17 at 11:04
  1. Use a function.
  2. Don't mutate data you don't want to be mutated.

    Rather than using list.extend, use list.__add__. However don't use list.__iadd__, as that has the same problem as list.extend.

  3. Learn how to copy Python objects. You can use list([1, 2, 3]) to make a copy of the list. You can also use the copy library too.

  4. Don't waste memory, just yield. If you need a list use list(product(...))
def product(input):
    for i in range(len(input)):
        for j in range(i+1, len(input)):
            yield input[i] + input[j]
| improve this answer | |
  • 1
    \$\begingroup\$ Yield is better then mine, memorywise, but don't use input as a variable name. :) \$\endgroup\$ – Ludisposed Nov 20 '17 at 11:32
  • \$\begingroup\$ @Ludisposed You could always use a generator, rather than list, comprehension. And shadowing built-ins is fine IMO. It's not like the OP's going to use the original input in the function. \$\endgroup\$ – Peilonrayz Nov 20 '17 at 11:34
  • \$\begingroup\$ I just try to avoid shadowing built ins, at all costs! Maybe just a bit paranoid :$ \$\endgroup\$ – Ludisposed Nov 20 '17 at 11:43
  • \$\begingroup\$ @Ludisposed There are good arguments for and good against. I don't think you'll confuse input as builtins.input in a small 4 lines of code function. If however it were a 200 line function I'd whole heartedly agree. I do admit it's not that great a variable name, but we don't have context to improve on that. \$\endgroup\$ – Peilonrayz Nov 20 '17 at 11:47
  • \$\begingroup\$ @Peilonrayz thanks for this, seems really simple! \$\endgroup\$ – Van Peer Nov 20 '17 at 11:50

What could be improved here?

@thnx Peilonrayz, a generator expression will work better on large inputs

  1. Use a nested generator expression.
  2. DON'T hardcode variables, this will make your code very static.
  3. Make a function, to accept different variables.

def custom_combo(L):
    return (L[i] + L[j] for i in range(len(L)) for j in range(i+1, len(L)))

if __name__ == '__main__':
    a = [[1, 2], [2, 3], [4, 3]]
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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