7
\$\begingroup\$

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)
\$\endgroup\$
6
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Also it might be worth it to have a look at the unique_everseen function in the itertools recipes, which has some performance improvements and an optional key by which to determine uniqueness (but is otherwise the same as your unique function). \$\endgroup\$ – Graipher Sep 30 at 7:40
  • 1
    \$\begingroup\$ Yeah, that's worth mentioning. I put it in. I'll keep my unique() around for ease spotting of what it does. \$\endgroup\$ – Gloweye Sep 30 at 7:44
  • 1
    \$\begingroup\$ Beware that it is just a recipe, though. Unfortunately you cannot just do from itertools import unique_everseen. \$\endgroup\$ – Graipher Sep 30 at 7:48
  • \$\begingroup\$ Ah, OK. Didn't pay attention to the header. \$\endgroup\$ – Gloweye Sep 30 at 7:50
  • 1
    \$\begingroup\$ I think dict.fromkeys(iterable) serves more or less the same functionality (for Python version >= 3.6) as unique(iterable). \$\endgroup\$ – GZ0 Sep 30 at 21:23

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.