1

Is there a way I can implement the code block below using map or list comprehension or any other faster way, keeping it functionally the same?

def name_check(names, n_val):
    lower_names = names.lower()
    for item in set(n_val):
        if item in lower_names:
            return True
    return False

Any help here is appreciated

new_c_user
  • 123
  • 2
  • 12
  • What data structures are names and n_val? – en_Knight May 18 '16 at 02:05
  • names is a str, n_val a list – new_c_user May 18 '16 at 02:06
  • I don't know for sure, but this question might be more suited for codereview if this code works. Any one know? – en_Knight May 18 '16 at 02:06
  • 1
    @en_Knight Correct, although it's missing a bit of context and description of what it's doing, and it's a bit poorly titled for a CR question (lots of questions could have that exact same title). See [*a guide to CR for SO users*](http://meta.codereview.stackexchange.com/q/5777/23788) for all the information. – Mathieu Guindon May 18 '16 at 02:14
  • Okay that sounds right. I'd recommend reading that link and posting there in the future with a more tailored question as Mat's Mug recommends, new_c_user, they give some great responses to this sort of question when it's well phrased – en_Knight May 18 '16 at 02:15
  • 1
    @en_Knight your answer below would do great on CR - where's your CR account? =) – Mathieu Guindon May 18 '16 at 02:17
  • So you have a string containing a single name and its called `names`? – Paul Rooney May 18 '16 at 02:23

1 Answers1

1

A simple implementation would be

return any(character in names_lower for character in n_val)

A naive guess at the complexity would be O(K*2*N) where K is the number of characters in names and N is the number of characters in n_val. We need one "loop" for the call to lower*, one for the inner comprehension, and one for any. Since any is a built-in function and we're using a generator expression, I would expect it to be faster than your manual loop, but as always, profile to be sure.

To be clear, any short-circuits, so that behaviour is preserved


Notes on Your Implementation

On using a set: Your intuition to use a set to reduce the number of checks is a good one (you could add it to my form above, also), but it's a trade-off. In the case that the first element short circuits, the extra call to set is an additional N steps to produce the set expression. In the case where you wind up checking each item, it will save you some time. It depends on your expected inputs. If n_val was originally an iterable, you've lost that benefit and allocated all the memory up front. If you control the input to the function, why not just recommend it's called using lists that don't have duplicates (i.e., call set() on its input), and leave the function general?


* @Neopolitan pointed out that names_lower = names.lower() should be called out of the loop, as your original implementation called it, else it may (will?) be called repeatedly in the generator expression

Community
  • 1
  • 1
en_Knight
  • 5,301
  • 2
  • 26
  • 46
  • Don't want to call names.lower() inside the generator expression; I don't think the interpreter can guarantee that names.lower() returns the same thing every time (it certainly doesn't for an arbitrary object names), so assign `lower_names = names.lower()` outside the loop as OP had it. It will also be faster. – Neapolitan May 18 '16 at 05:46