8
\$\begingroup\$

This is the "Clean up the words" challenge from CodeEval:

Challenge

Given a list of words mixed with extra symbols. Write a program that will clean up the words from extra numbers and symbols.

Specifications

  1. The first argument is a path to a file.
  2. Each line includes a test case.
  3. Each test case is a list of words.
  4. Letters are both lowercase and uppercase, and mixed with extra symbols.
  5. Print the words separated by spaces in lowercase letters.

Constraints

  1. The length of a test case together with extra symbols can be in a range from 10 to 100 symbols.
  2. The number of test cases is 40.

Input Sample

(--9Hello----World...--)  
Can 0$9 ---you~  
13What213are;11you-123+138doing7

Output Sample

hello world  
can you  
what are you doing

In a previous question someone joked the program would be much shorter / simpler in Python. I accepted this as a challenge and excuse to practice Python.

Solution:

import sys
import re

def sanitized(line):
    sanitized_line = re.sub("[^a-zA-Z]+", " ", line)
    return sanitized_line.lower().strip()

def main(file):
    with open(file, 'r') as input_file:
        for line in input_file:
            print(sanitized(line))

if __name__ == "__main__":
    try:
        file = sys.argv[1]
        main(file)
    except:
        print("No argument provided.")

It's so succinct I'm unsure there's enough for a review, but this site surprised me in the past.

\$\endgroup\$
  • \$\begingroup\$ Well, it looks nice. As an alternative to that regex you could also use re.sub(r'\W+', '', line). Apparently I was about to suggest ''.join(ch for ch in string.printable if ch.isalnum()) but in this case using a regex is far better when talking to the speed of processing. \$\endgroup\$ – Grajdeanu Alex. Jun 12 '16 at 20:08
  • 1
    \$\begingroup\$ The W regex shortcut won't apply since this also removes numbers. For the same reason isalpha is preferable over isalnum \$\endgroup\$ – Legato Jun 12 '16 at 20:10
  • 1
    \$\begingroup\$ You're right. Talking about the speed of you program, try re.compile before actually subing. IT saves a few usecs. Have a quick look here \$\endgroup\$ – Grajdeanu Alex. Jun 12 '16 at 20:13
7
\$\begingroup\$

The code is pretty clear and clean. I will have to do some nit-picking, but here goes an attempt:

  • try - except without the actual exception(s). Here, you print an error when the exception raised is an IndexError (to sys.argv), but consider that you get the same error message if the file can't be properly opened (or is simply non-existent).

    Change it to except IndexError, and let any IO/OSError just bubble up, since the corresponding error message is often clear enough (e.g., 'Is a directory', 'File does not exist', etc).

  • You're printing the error message to stdout. You could consider exiting using sys.exit("No argument provided."), which will exit with an exit code of 1, and print the message to stderr. Or raise an equivalent exception to the one you're catching.

  • 'r'ead mode is the default opening mode. While perhaps clearer with the extra argument, with open(file) as ... is more standard.

  • In case of Python 2: don't use file as a variable name, since it shadows the built-in file function. Use e.g. filename instead.

Nits:

  • Do you want it to be Python 3 only? Otherwise, include a from __future__ import print_function at the top. print will still work as it is now in Python 2, but once you use more arguments, you're printing a tuple in Python 2, instead of a series of concatenated str-ified elements. (Thanks to JoeWallis in the comments for pointing out the mistake that I was thinking "tuple"-wise where it was just a parenthesized string.)

  • doc-strings to the program and functions, if you want to go all-out. (A linter would complain about that, even if it's pretty unnecessary here, and not to the point of the exercise.)

\$\endgroup\$
  • \$\begingroup\$ Related to your try...except remarks, one should almost never use bare except: as per PEP8: A bare except: clause will catch SystemExit and KeyboardInterrupt exceptions, making it harder to interrupt a program with Control-C, and can disguise other problems. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:). It's not a problem here, but one should make it a habit not use bare except:. \$\endgroup\$ – Vedran Šego Jun 13 '16 at 13:22
  • \$\begingroup\$ @VedranŠego I state it less general (more specific about catching the proper IndexError, since that relates to the error message), but aren't we saying the same thing; or do I miss something? Or is your comment for the question, not my answer? \$\endgroup\$ – user86624 Jun 14 '16 at 4:24
  • \$\begingroup\$ @Evert I was in doubt whether to put it under the question or here. I opted here, as an additional comment to what you wrote, aimed for the OP. \$\endgroup\$ – Vedran Šego Jun 14 '16 at 9:37

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.