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?
asked Nov 23, 2016 at 17:25
\$\endgroup\$

2 Answers 2

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.

answered Nov 23, 2016 at 18:28
\$\endgroup\$
3
  • \$\begingroup\$ Only problem, if you don't strip the numbers it will have whitespace in it which will in turn raise a ValueError \$\endgroup\$ Commented Nov 23, 2016 at 19:05
  • 1
    \$\begingroup\$ @papasmurf Have You tried using a single strip instead of many? \$\endgroup\$ Commented Nov 23, 2016 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\$ Commented Nov 23, 2016 at 23:25
1
\$\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()))))
answered Nov 24, 2016 at 10:14
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.