10
\$\begingroup\$

This is a warm-up programming exercise for learning string methods in Python from HackerRank:

You are given a string S. Your task is to find if string S contains: alphanumeric characters, alphabetical characters, digits, lowercase and uppercase characters.

Input Format

Single line containing, string S.

Constraints

0 < len(S) < 1000

Output Format

  • In First line, print True if S has alphanumeric character otherwise print False.
  • In Second line, print True if S has alphabetical character otherwise print False.
  • In Third line, print True if S has digits otherwise print False.
  • In Fourth line, print True if S has lowercase character otherwise print False.
  • In Fifth line, print True if S has uppcase character otherwise print False.

My working solution is here:

def parse_input():
    string = raw_input()
    return string

def check_string(string):
    check_funs = [str.isalnum,
                  str.isalpha,
                  str.isdigit,
                  str.islower,
                  str.isupper,
                  ]
    return [any(fun(char) for char in string) for fun in check_funs]

def print_output(results):
    for el in results:
        print el

if __name__ == '__main__':
    string = parse_input()
    print_output(check_string(string))

Apologies for the lack of docstrings; I think aside from that, I'm PEP8 compliant but please let me know if I missed something. I'm interested in any and all feedback of course, but in particular:

  • I feel like my check_string() function isn't very pretty. What's the preferred syntax for folding a list of class methods into a list comprehension? I found a possibly relevant Stack Overflow question but couldn't quite make heads or tails of it. It just doesn't seem right to have to do str.method to refer to a function, but then call it with fun(my_string) for fun in func_list instead of my_string.fun() for fun in func_list. (But that latter bit doesn't work of course.)

What are some better ways?

\$\endgroup\$

3 Answers 3

3
+50
\$\begingroup\$

Add another function to simplify your list comprehension

You can make your code more readable by creating a function to simplify your list comprehension. By defining an additional function like so :

def func_on_str(string, func):
    return any(func(char) for char in string)

you can rewrite the return statement of check_string more clearly:

return [func_on_str(string, func) for func in check_funcs]

(you can see I agree with @SuperBiasedMan that your variable names could be clearer)

Use getattr() and the code gets shorter but not necessarily more readable

If you want to run member functions, here's what you can do ignoring SRP...it is nice and short. I think it's readable, but others may disagree.

check_funcs = ["isalnum",
                  "isalpha",
                  "isdigit",
                  "islower",
                  "isupper",
                  ]

for func in check_funcs:
    print(any(getattr(char, func)() for char in string))

That could be your whole main() function if you wrote one. Or if you want to keep a separate function for printing you can do:

result_list = []
for func in check_funcs:
    result_list.append(any(getattr(char, func)() for char in string))
return result_list

or of course you could use a monster list comprehension:

   return [any(getattr(char, func)() for char in string) for func in check_funcs]

But I am not sure any of these versions are more readable than what you've got already.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks. The getattr syntax was what I was looking for. Now that I see it I agree it may not be any better; I guess python is not always as pythonic as one would hope! \$\endgroup\$
    – Curt F.
    Sep 9, 2015 at 22:49
  • \$\begingroup\$ I liked all of the answers but am giving you the bounty since this getattr syntax was what I was looking for. I agree it doesn't make the code much more readable in the end.... \$\endgroup\$
    – Curt F.
    Sep 10, 2015 at 12:30
  • \$\begingroup\$ @CurtF. Thank you! Yes I very much agree with your comment that Python is not always as Pythonic as one would expect. But I think I learn the most when I am trying to at least figure out whether a Pythonic solution exists. \$\endgroup\$
    – sunny
    Sep 10, 2015 at 14:06
4
\$\begingroup\$

Your code is fine. I don't know what you're so worried about. But there's just some unnecessary stuff. For example:

def parse_input():
    string = raw_input()
    return string

What is the point of that function? Well, it's basically:

parse_input = raw_input

Which is to say, just don't have it. Additionally, your list-comprehension is unnecessary:

return [any(fun(char) for char in string) for fun in check_funs]

That builds up the whole list before we then print them one by one. We don't need the whole list in one go, so we can just return a generator:

return (any(...) for fun in check_funs)

Or we could just print as we go:

for fun in check_funs:
    print any(fun(c) for c in string)

Just doing that lets us reduce two completely unnecessary functions into:

def process_string(string):
    check_funs = (str.isalnum,
                  str.isalpha,
                  str.isdigit,
                  str.islower,
                  str.isupper,
                  )
    for fun in check_funs:
        print any(fun(char) for char in string)

if __name__ == '__main__':
    process_string(raw_input())

Refactor when you need to, otherwise you're just overengineering everything. Should the check_funs be an argument? What if we need at least 2 to match the func instead of 1? There's so many arbitrary other features we could add - so let's worry about those only when they need to.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot. I like the idea to make check_funs an argument. \$\endgroup\$
    – Curt F.
    Sep 9, 2015 at 22:50
4
\$\begingroup\$

I think your way of calling the methods is just fine. Readable and succinct. My problem is with the names check_funs and fun. Why are we having fun here?

Generally it's best not to use a function or an abbreviation of functions as a name. At the very least, use func as a much clearer abbreviation for function. Even though it is a somewhat relevant name here, it's not important that they're functions. What matters is what they do or what they're for. I'd cut it down to just checks and then for check in checks. Even aside from that though, make sure an abbreviation is clear and not just removing letters. If I was asked what fun was short for, function would be low down on my list. I could only follow in this instance because I knew that you were actually putting functions in the list.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.