Bsrgvty

Problem

Write a function that replaces the words in raw with the words in code_words such that the first occurrence of each word in raw is assigned the first unassigned word in code_words. If the code_words list is too short, raise an error. code_words may contain duplicates, in which case the function should ignore/skip them.

Examples:

encoder(["a"], ["1", "2", "3", "4"]) → ["1"]
encoder(["a", "b"], ["1", "2", "3", "4"]) → ["1", "2"]
encoder(["a", "b", "a"], ["1", "1", "2", "3", "4"]) → ["1", "2", "1"]

Solution

def encoder(raw, code_words):
 cw = iter(code_words)
 code_by_raw = # map of raw item to code item
 result = []
 seen = set() # for ignoring duplicate code_words
 for r in raw:
 if r not in code_by_raw:
 for code in cw: # cw is iter(code_words), "persistent pointer"
 if code not in seen:
 seen.add(code)
 break
 else: # nobreak; ran out of code_words
 raise ValueError("not enough code_words")
 code_by_raw[r] = code
 result.append(code_by_raw[r])
 return result

Questions

My main concern is the use of cw as a "persistent pointer". Specifically, might people be confused when they see for code in cw?

What should be the typical best practices in this case?

Might it be better if I used the following instead?

try:
 code = next(cw)
 while code in seen:
 code = next(cw)
except StopIteration:
 raise ValueError("not enough code_words")
else:
 seen.add(code)

My main concern is the use of cw as a "persistent pointer". Specifically, might people be confused when they see for code in cw?

No. Instead, you can just remove the line cw = iter(code_words) as long as it's a native iterable. "Persistent Pointer" isn't a thing in python, because all python knows are Names.

What should be the typical best practices in this case?

That would be building a dictionary and using it for the actual translation. You're basically already doing this with your code_by_raw, if a bit more verbose than others might. The only real difference would be that, in my opinion, it would be better to first establish the translation, and then create the result.

Except for your premature result generation, I would say your current function isn't bad. It does what it needs to do, it does it well without stupid actions, but it's not very readable. It's said often, I think you need to factor out a bit of code. Specifically, the bit that handles the fact that your inputs don't have to yield unique values, and how you need to handle duplicates.

I would suggest a generator to handle that. This simplifies the main function a ton. (A comment pointed me towards the unique_everseen recipe, which is a slightly broader function. We don't quite need all it's functionality, but it might be worth the effort if you need some more flexibility.)

def unique(iterable):
 """ Generator that "uniquefies" an iterator. Subsequent values equal to values already yielded will be ignored. """
 past = set()
 for entry in iterable:
 if entry in past:
 continue
 past.add(entry)
 yield entry

def encoder(raw_words, code_words):
 # Create mapping dictionary:
 code_by_raw = dict(zip(unique(raw_words), unique(code_words))
 # Check if we had sufficient code_words:
 if len(code_by_raw) < len(raw_words):
 raise ValueError("not enough code_words")
 # Do translation and return the result
 return [code_by_raw[raw] for raw in raw_words]

I can't completely tell your experience level with python. For result creation, I'm using comprehensions here.

Might it be better if I used the following instead?

It would not be bad functionally to use a structure like that, but it's still ugly (but opinions may differ). It basically does the same as my unique() generator up there.