6
\$\begingroup\$

I'm doing a CodeEval challenge, where the object is to find a famous writers name and a year, inside of a random given string.

You have a set of rows with names of famous writers encoded inside. Each row is divided into 2 parts by pipe char (|). The first part has a writer's name. The second part is a "key" to generate a name.

Your goal is to go through each number in the key (numbers are separated by space) left-to-right. Each number represents a position in the 1st part of a row. This way you collect a writer's name which you have to output.

Challenge input:

osSE5Gu0Vi8WRq93UvkYZCjaOKeNJfTyH6tzDQbxFm4M1ndXIPh27wBA rLclpg| 3 35 27 62 51 27 46 57 26 10 46 63 57 45 15 43 53

3Kucdq9bfCEgZGF2nwx8UpzQJyHiOm0hoaYP6ST1WM7Nks5XjrR4IltBeDLV vA| 2 26 33 55 34 50 33 61 44 28 46 32 28 30 3 50 34 61 40 7 1 31

Expected output:

Stephen King 1947

Kyotaro Nishimura 1930


I've successfully solved this problem and would like some critique on my work:

import sys


def find_a_writer(string, int_list):
    """ Find a famous writer and a year in a random string
    >>> print(find_a_writer("osSE5Gu0Vi8WRq93UvkYZCjaOKeNJfTyH6tzDQbxFm4M1ndXIPh27wBA rLclpg",
    >>> ['3', '35', '27', '62', '51', '27', '46', '57', '26', '10', '46', '63', '57', '45', '15', '43', '53']))
    Stephen King 1947"""
    results = []
    string_list = list(string)
    for i in int_list:
        results.append(string_list[int(i) - 1])
    return ''.join(results)


if __name__ == '__main__':
    with open(sys.argv[1]) as data:
        for line in data.readlines():
            data_arr = line.rstrip().strip().split("|")
            print(find_a_writer(data_arr[0].strip(), data_arr[1].strip().split(" ")))

What I would like to know:

  1. What did I do well?
  2. Is there anything I can do better?
  3. Can I shorten the function?
\$\endgroup\$
6
\$\begingroup\$

3) Yes, you can shorten the function:

def decide(mess, indexes):
    return ''.join(mess[int(i)-1] for i in indexes)

I just removed the unnecessary conversion to list and replaced the append loop with a generator expression.

Tuple unpacking allows more documenting names:

mess, indexes = lunedì.split('|')
print(decoder(mess, indexes.split()))

I do not think you need so much stripping at the file parsing step.

\$\endgroup\$
  • \$\begingroup\$ Only problem, if you don't strip the numbers it will have whitespace in it which will in turn raise a ValueError \$\endgroup\$ – papasmurf Nov 23 '16 at 19:05
  • 1
    \$\begingroup\$ @papasmurf Have You tried using a single strip instead of many? \$\endgroup\$ – Caridorc Nov 23 '16 at 20:43
  • \$\begingroup\$ @papasmurf Except that split() without arguments has extra logic to cleanup the string and handle repeated blank characters. So ' 1 2 3 '.split() is ['1', '2', '3']. \$\endgroup\$ – Mathias Ettinger Nov 23 '16 at 23:25
  • \$\begingroup\$ @MathiasEttinger Did not know that, thank you \$\endgroup\$ – papasmurf Nov 24 '16 at 18:44
0
\$\begingroup\$

I would not go for this in production since this code is pretty hard to understand by just looking on it. But just for fun and education you could use python built-in tools to solve this in 1 line.

import operator
from functools import partial

a = "osSE5Gu0Vi8WRq93UvkYZCjaOKeNJfTyH6tzDQbxFm4M1ndXIPh27wBA rLclpg| 3 35 27 62 51 27 46 57 26 10 46 63 57 45 15 43 53"

print(''.join(map(partial(operator.getitem, a), 
                  map(lambda x: int(x)-1, 
                      a.split('|')[1].split()))))
\$\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.