These words are so dirty

Question

Challenge:

Write a program that will clean up the words from extra numbers and symbols.

Specifications:

The first argument is a path to a file.
Each line includes a test case with a list of words: letters are both lowercase and uppercase, and are mixed with extra symbols.
Print the words separated by spaces in lowercase letters.
The length of a test case together with extra symbols can be in a range from 10 to 100 symbols. The number of test cases is 40.

Solution:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.Scanner;

public class CleanWords {
    public static void main(String[] args) {
        try (Scanner input = new Scanner(new File(args[0]))) {
            while(input.hasNextLine()) {
                System.out.println(sanitize(input.nextLine()));
            }
        } catch (FileNotFoundException fnfe) {
            System.out.println(args[0] + " doesn't exist");
        }
    }

    private static String sanitize(String input) {
        StringBuilder result = new StringBuilder();
        for (String s : input.split("[^a-zA-Z]+")) {
            result.append(' ').append(s);
        }
        return result.toString().toLowerCase().trim();
    }
}

Sample Input:

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

Sample Output:

hello world  
can you  
what are you doing

I'm wondering if:

1. It would be more efficient to call trim within the loop?
2. If there's a less costly way than looping twice between calling both toLowerCase and trim

These may seem like givens, but it's been a long time since I did one of these, and I wanted to get back into it with a seemingly simple one. I passed on the first try which I largely have CR to thank for. For those interested, the source is CodeEval.com.

Answer 1

This can be significantly cleaned up and simplified using Java 8. Let's start with a quick review, though before rewriting it all :)

public static void main(String[] args) {
    try (Scanner input = new Scanner(new File(args[0]))) {

You perform no validation whatsoever here. When this is called without an argument, you'll get a ArrayIndexOutOfBoundsException... not nice. Validate your inputs (even when the assignment says you'll get a path)

Also you should consider using JSR-203 (aka. java.nio) API. It's cleaner and more informative wrt. Exceptions and general handling of problematic inputs. Also it integrates very nicely with Streams :D

But you're using try-with-resources, which is a good start.

        while(input.hasNextLine()) {
            System.out.println(sanitize(input.nextLine()));
        }

This is a small indication your main-method is doing too much. You do:

• Open file
• read that file
• call sanitize
• print
• error handling

Following the Single Responsibility Principle by the letter would mean every single one of these should be a separate method. I personally find that overkill, but I'd at least shift the responsibilities a little by separating result retrieval (aka. sanitize()) from printing.

Which gets me to sanitization or rather how you do it:

    for (String s : input.split("[^a-zA-Z]+")) {

What you're doing here is interesting and basically a cool trick, but.. you're cheating around what you actually want.

You want to extract words consisting of [a-z] (case insensitive). What you do is split you String on everything else and then reassemble it.

So there's a few things you could've done instead. First possibility would've been to extract the words with a Pattern and then assembling them, which might've been more obvious:

Pattern words = Pattern.compile("([a-z]+)", Pattern.CASE_INSENSITIVE);
Matcher m = words.matcher(s);
StringBuilder sb = new StringBuilder();
while (m.find()) {
   sb.append(m.group()).append(' ');
}

This basically accomplishes the same as your code, but is more explicit as to what you want to match. But this is a lot of code for something dead simple. It boils down to what your split does in combination with the loop.

What you do is replace everything, that's not a latin letter with a space. Why not write that?

s.replaceAll("[^a-zA-Z]+", " ");

this eliminates the for-loop you have and .. basically does the same :D

Now that we got this out of the way, let's look at how it looks with JSR203 and Streams :)

public static void main(String[] args) {
    try (Stream<String> lines = Files.lines(Paths.get(args[0]))) {
        final String output = lines.map(CleanWords::sanitize)
             .collect(Collectors.joining(System.lineSeparator()));
        System.out.println(output);
    } catch (IOException ex) {
        System.out.println("Could not read File");
    }
}

private static String sanitize(String input) {
     return input.replaceAll("[^A-Za-z]+", " ").toLowerCase().trim();
}

Answer 2

Regarding naming:

My first thought about a class named CleanWords without looking at its JavaDoc or implementation would be: "This class could implement Collection or extend one of the collection classes to contain clean words.".

My second thought after seeing the beginning of the class declaration without any of these: "This looks more like a name of a method that cleans words.".

I'd name the class WordCleaner and the method cleanUp(). Such also better reflecting the challenge description.