Finding associations for a set of keys

Question

I am writing my first programs in Python and want to be sure my code is good and not too C++-like. Consider the problem of finding all common 'associations' for set of 'keys'.

Data format is

• first line: integer N
• N lines: key: association 1 ... association P_1
• N+2th line: integer K
• K lines: key 1 ... key T_1

For each line after K, the program should output all common associations for all keys listed.

For example, the correct output for input

10
0: even
1: odd
2: even prime
3: odd prime
4: even composite notprime
5: odd prime
6: even composite notprime
7: odd prime
8: even composite notprime
9: odd composite notprime
2
8 4
2 3

is

composite even notprime
prime

My code is

f = open('input.txt', 'r') #input file
d = {} #associations
uni = set() #set of all associations
for i in range(int(f.readline())):
 t = f.readline().strip().split(': ') #split into list ('key', 'assoc1 ... assoc P_1')
 d[t[0]] = s = set(t[1].split(' ')) #d : key -> set('assoc1', ..., 'assoc P_1')
 uni |= s #add to uni
for i in range(int(f.readline())):
 #read keys as list and intersect uni and list's associations
 print reduce(lambda assoc, key: assoc & d[key], set(f.readline().strip().split(' ')), uni.copy())

How good is my code from the viewpoint of experienced Python programmers?

Answer 1

Your code looks pretty good for a newcomer to the language. Some comments:

• I don't quite understand why you need uni.
• References to t[0] and t[1]: It's more declarative to unpack tuples/lists and give names to its components -> something, another_thing = t
• for i in range(int(f.readline())). I won't say this is bad, but certainly not very declarative. Note that itertools.islice can be used to read N lines from a file object -> itertools.islice(f, 10).
• Python2: Unless you have a compelling reason, use Python 3.
• reduce(lambda.... As much as I like functional style, code that uses reduce with long lambdas usually looks cryptic. But I think it's still ok to use reduce with named functions.
• int(f.readline()). Personally I like to give names to values when it's not clear what they stand for. Granted, this may increase the LOC slightly, but I think it's worthwhile. In this case, if we write number_of_queries = int(f.readline()) and later use the variable, it's easy to see what's going on.

As a first refactor, I'd write:

import itertools
import functools
lines = open("input.txt")
# Parse associations
n_associations = int(lines.readline())
associations = {}
for line in itertools.islice(lines, n_associations):
 n, associations_string = line.split(":")
 associations[n] = set(associations_string.split())
# Parse and run queries
n_queries = int(lines.readline())
for line in itertools.islice(lines, n_queries):
 numbers = line.split()
 common = functools.reduce(set.intersection, (associations[n] for n in numbers))
 print(" ".join(common))

Ok, now let's take it a step further. Firstly, there are no functions at all, no modularization, that's no good. Secondly, I prefer a functional approach and there is some imperative stuff going on there. That's how I would write it in a more functional and declarative style:

def parse_and_execute_queries(path):
 def parse_association(line):
 number, associations_string = line.split(":")
 return (number, set(associations_string.split()))
 def execute_query(associations, query_string):
 numbers = query_string.split()
 common = functools.reduce(set.intersection, (associations[n] for n in numbers))
 return " ".join(common)
 lines = open(path)
 n_associations = int(lines.readline())
 association_lines = itertools.islice(lines, n_associations)
 associations = dict(parse_association(line) for line in association_lines) 
 n_queries = int(lines.readline())
 queries_lines = itertools.islice(lines, n_queries)
 return (execute_query(associations, line) for line in queries_lines)
for query_result in parse_and_execute_queries("input.txt"):
 print(query_result)

• Jay Foreman: I needed uni because I didn't know set.intersect. Thank you for the reply!