Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

All the static fields of your class can be made into local variables. This gives them a smaller scope. Always give variables the smallest possible scope. When you do this you'll notice userString and al are already defined as a local variable. al should be declared as a List<String>al should be declared as a List<String>, in fact you'll see that the code works fine with the List returned by Arrays.asList().

All the static fields of your class can be made into local variables. This gives them a smaller scope. Always give variables the smallest possible scope. When you do this you'll notice userString and al are already defined as a local variable. al should be declared as a List<String>, in fact you'll see that the code works fine with the List returned by Arrays.asList().

All the static fields of your class can be made into local variables. This gives them a smaller scope. Always give variables the smallest possible scope. When you do this you'll notice userString and al are already defined as a local variable. al should be declared as a List<String>, in fact you'll see that the code works fine with the List returned by Arrays.asList().

Source Link
bowmore
  • 5.4k
  • 1
  • 20
  • 23

Scoping

All the static fields of your class can be made into local variables. This gives them a smaller scope. Always give variables the smallest possible scope. When you do this you'll notice userString and al are already defined as a local variable. al should be declared as a List<String>, in fact you'll see that the code works fine with the List returned by Arrays.asList().

Naming

Use meaningful names instead of abbreviations for names : al, sc, even temp don't convey anything.

Algorithm

Testing whether the first letter of a String is a vowel shouldn't require you to uppercase the entire String. You shouldn't even be making a second String (tester). It's easier to make a String of all vowels (upper and lower case) and use indexOf() on that string with the character you want to test.

The while(true) loop is bogus too. I had to look for the exit condition. Make it explicit.

You remember to close the Scanner, but do it in a finally block, so a possible Exception doesn't bypass the closing. For extra points use a try-with-resources structure (see my refactored example).

Wait, remember I said the code would work fine with the result of Arrays.asList()? It will, but you don't even need a list to loop over an array. Just use an advanced for loop.

for (String temp : userString.split("\\s")) {
    ...
}

latinString should be a StringBuilder, as it's mostly used to build the resulting String.

Use helper methods to explain what you're doing or to extract repeated code.

Here's my refactored version :

public static void main(String[] args) {
    try (Scanner scanner = new Scanner(System.in)) { // try with resource will ensure the Scanner gets closed.
        String userString = getUserInput(scanner);
        while (!userString.isEmpty()) {
            System.out.println(buildLatinString(userString));
            userString = getUserInput(scanner);
        }
    }
}

private static String buildLatinString(String userString) {
    StringBuilder latinString = new StringBuilder();
    for (String word : userString.split("\\s")) {
        latinizeWord(latinString, word);
    }
    return latinString.toString();
}

private static void latinizeWord(StringBuilder latinString, String word) {
    if (startsWithVowel(word)) {
        latinString.append(word).append("ay ");
    } else {
        latinString.append(word.substring(1)).append(word.charAt(0)).append("ay ");
    }
}

private static String getUserInput(Scanner scanner) {
    System.out.print("Enter a string to be converted into Pig Latin. To stop, enter a blank input: ");
    return scanner.nextLine();
}

private static boolean startsWithVowel(String word) {
    return "AEIOUaeiou".indexOf(word.charAt(0)) != -1;
}