1
\$\begingroup\$

I created a script to perform a union on 2 lists: keys and words. I needed to create a dictionary from them. If there are more keys than words, missing words are replaced with "None". Any excess words are removed.

How can I improve this code to be more compact and perform better?

#! /usr/bin/env python3
import random
import string
from itertools import zip_longest
def generator(listKeys,listWords):

    diff = len(listKeys)-len(listWords)
    if diff > 0:
        for i in range(0,diff):
            listWords.append(None)
    elif diff<0:
        listWords = listWords[0:len(listKeys)]

    done = dict(zip(listKeys,listWords))
    return done

if __name__ == "__main__":
    listKeys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n']
    listWords = [94, 87, 92, 5, 75, 91, 60, 5]
    print("Keys more values:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))

    listKeys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']
    listWords = [9, 65, 18, 80, 2, 16, 81, 98, 73, 2, 88, 30, 1, 78, 92]
    print("Values more keys:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))

    listKeys = []
    listWords = []

    for i in range(0,random.randrange(5,15)):
        listKeys.append(list(string.ascii_lowercase)[i])

    for i in range(0,random.randrange(6,16)):
        listWords.append(random.randrange(0,100))

    print("Random:\nKeys: %s \nValues %s\nResult: %s\n" % (listKeys, listWords,generator(listKeys,listWords)))
\$\endgroup\$
1
  • \$\begingroup\$ thanks for editing. English is no my native language \$\endgroup\$ – Tutunak Oct 2 '14 at 11:25
2
\$\begingroup\$

zip and itertools.zip_longest do exactly what you are looking for, you only need to choose between the two depending on the lengths of the lists:

def generator(listKeys, listWords):
    if len(listKeys) > len(listWords):
        zipped = zip_longest(listKeys, listWords)
    else:
        zipped = zip(listKeys, listWords)
    return dict(zipped)

This can even be shortened to

def generator(listKeys, listWords):
    my_zip = zip_longest if len(listKeys) > len(listWords) else zip
    return dict(my_zip(listKeys, listWords))

Note that generator is not a good name for this function as the word generator has a particular meaning in Python.

\$\endgroup\$
1
\$\begingroup\$

Style and tools

Your code looks weird : you have line breaks in unexpected places but not in a few expected places (such as before defining a function), whitespaces are not consistent. In Python, you'll find PEP8, a style guide that can be quite helpful to you. Even better, you'll also find tools to detect problems such as pep8 and to fix them such as autopep8.

You'll also find various other tools to check your code such as pylint or pychecker and pyflakes. They can be useful to find problems in your code. For instance, lists2.py:4: 'zip_longest' imported but unused and Unused variable 'i' (unused-variable) (for your information, _ is the usual name for throw-way variables in Python).

Finally, your variable names do not follow the convention.

Once this is done, your code looks like :

#! /usr/bin/env python3
"""Docstring for the module."""
import random
import string


def generator(keys, words):
    """Docstring for the function."""
    diff = len(keys) - len(words)
    if diff > 0:
        for _ in range(0, diff):
            words.append(None)
    elif diff < 0:
        words = words[0:len(keys)]

    done = dict(zip(keys, words))
    return done

if __name__ == "__main__":
    keys = ['a', 'b', 'c', 'd', 'e', 'f',
            'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n']
    words = [94, 87, 92, 5, 75, 91, 60, 5]
    print("Keys more values:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

    keys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']
    words = [9, 65, 18, 80, 2, 16, 81, 98, 73, 2, 88, 30, 1, 78, 92]
    print("Values more keys:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

    keys = []
    words = []

    for i in range(0, random.randrange(5, 15)):
        keys.append(list(string.ascii_lowercase)[i])

    for i in range(0, random.randrange(6, 16)):
        words.append(random.randrange(0, 100))

    print("Random:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

Making things more simple

First, range(0, foo) can be replaced by range(foo).

Then, you can replace x = []; for i in bar: x.append(foo()) with list comprehension : x = [foo() for i in bar].

Your code becomes :

keys = [list(string.ascii_lowercase)[i] for i in range(random.randrange(5, 15))]
words = [random.randrange(0, 100) for i in range(random.randrange(6, 16))]

Even better, you probably don't need to get letters individually from string.ascii_lowercase. You can just use list(string.ascii_lowercase[:X]).

Also, we don"t really need keys to be a list. At the end of the day, we just want it to be iterable and strings are iterable.

keys = string.ascii_lowercase[:random.randrange(5, 15)]
words = [random.randrange(0, 100) for _ in range(random.randrange(6, 16))]

Finally, you don't need to store your value in a variable to return it.

Your code now looks like :

#! /usr/bin/env python3
"""Docstring for the module."""
import random
import string


def generator(keys, words):
    """Docstring for the function."""
    diff = len(keys) - len(words)
    if diff > 0:
        for _ in range(diff):
            words.append(None)
    elif diff < 0:
        words = words[0:len(keys)]

    return dict(zip(keys, words))

if __name__ == "__main__":
    keys = ['a', 'b', 'c', 'd', 'e', 'f',
            'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n']
    words = [94, 87, 92, 5, 75, 91, 60, 5]
    print("Keys more values:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

    keys = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']
    words = [9, 65, 18, 80, 2, 16, 81, 98, 73, 2, 88, 30, 1, 78, 92]
    print("Values more keys:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

    keys = string.ascii_lowercase[:random.randrange(5, 15)]
    words = [random.randrange(0, 100) for _ in range(random.randrange(6, 16))]

    print("Random:\nKeys: %s \nValues %s\nResult: %s\n" %
          (keys, words, generator(keys, words)))

Other comments have been given about the code you actually care about :-)

Likely to be a bug

Once probably wouldn't expect neither keys, nor words to be affected by your function. However, words might get updated. This is definitly something you should try to avoid.

You could write something like :

def generator(keys, words):
    """Docstring for the function."""
    diff = len(keys) - len(words)
    if diff > 0:
        words = words + [None] * diff
    elif diff < 0:
        words = words[0:len(keys)]

    return dict(zip(keys, words))
\$\endgroup\$

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.