Code efficiency - Piglatin

Question

My code works and does what it is supposed to. However it needs to be more efficient. Could anyone help me tidy this up and show me where I could perhaps not rely on large loops (which it does a lot and slows it down).

import java.util.ArrayList;
public class PigLatin {
 public String translate(String str) {
 final String vowels = "aeiouAEIOU";
 ArrayList<Character> strArray = new ArrayList<Character>();
 for (int i = 0; i < str.length(); i++) { 
 strArray.add(str.charAt(i));
 }
 // if any digits are found in str, return null
 for (int i = 0; i < strArray.size(); i++) {
 char current = strArray.get(i);
 if (Character.isDigit(current)) {
 return null;
 }
 }
 // if str begins with a vowel
 for (int vowel = 0; vowel < vowels.length(); vowel++) {
 if (strArray.get(0) == vowels.charAt(vowel)) {
 return str + "way";
 }
 }
 // if str does not begin with a vowel, pass each letter to the end of str
 // until a vowel is found and add "ay" to the end.
 boolean isAVowel = false;
 int count = 0;
 while (!isAVowel) {
 char current = strArray.get(count);
 if (current != 'a' && current != 'e' && current != 'i' && current != 'o' && current != 'u' && current != 'A' && current != 'E' && current != 'I'
 && current != 'O' && current != 'U') {
 strArray.add(strArray.size(), current);
 strArray.remove(count);
 } else {
 isAVowel = true;
 }
 }
 strArray.add(strArray.size(), 'a');
 strArray.add(strArray.size(), 'y');
 String piglatinate = "";
 for (char l : strArray) {
 piglatinate = piglatinate + l;
 }
 piglatinate = piglatinate.toLowerCase();
 return piglatinate;
 }
}

Answer 1

To tidy up your code you need to divide your code in more manageable and readable methods. Now I'll give you a better idea of the algorithm.

FYI: I'll ommit every validity check and is left for exercise and I've used StringJoiner which is a Java 8 feature.

The algorithm

Your method is receiving a sentence which is composed of words. You are only able to "pig latinize" a word. The cleanest way to do this is to split your String into words. To simplify things you should tranlate your sentence to lower case first.
Iterate over words and translate them to pig latin and concatenate the results.

public static String encode(String s) {
 String[] words = WORD_SPLIT_PATTERN.split(s.toLowerCase());
 StringJoiner pigLatin = new StringJoiner(WORD_DELIMITER);
 for (String word : words) {
 pigLatin.add(pigLatinize(word));
 }
 return pigLatin.toString();
}

Next we'll look into pigLatinize method which receives a word. There are three cases how a word can look like.

Word...

1. is empty
   • return the empty word
2. starts with a vowel
   • return word and append -way to it
3. starts with a consonant
   • find the first vowel in the word. Substring the first part and append it to the second part. Finally append -ay to the new word.

Then the method is again simple.

private static String pigLatinize(String word) {
 if (word.isEmpty()) {
 return word;
 } else if (isVowel(word.charAt(0))) {
 return word + SYLLABLE_DELIMITER + PIG_LATIN_VOWEL_ENDING;
 } else {
 for (int i = 0; i < word.length(); i++) {
 if (isVowel(word.charAt(i))) {
 return word.substring(i) + SYLLABLE_DELIMITER + word.substring(0, i) + PIG_LATIN_ENDING;
 }
 }
 return word + SYLLABLE_DELIMITER + PIG_LATIN_ENDING;
 }
}

Finally let's bring everything together to a complete example:

class PigLatin {
 private static final String PIG_LATIN_ENDING = "ay";
 private static final String PIG_LATIN_VOWEL_ENDING = "way";
 private static final Pattern WORD_SPLIT_PATTERN = Pattern.compile("\\s+");
 private static final String WORD_DELIMITER = " ";
 private static final String SYLLABLE_DELIMITER = "-";
 public static String encode(String s) {
 String[] words = WORD_SPLIT_PATTERN.split(s.toLowerCase());
 StringJoiner pigLatin = new StringJoiner(WORD_DELIMITER);
 for (String word : words) {
 pigLatin.add(pigLatinize(word));
 }
 return pigLatin.toString();
 }
 private static String pigLatinize(String word) {
 if (word.isEmpty()) {
 return word;
 } else if (isVowel(word.charAt(0))) {
 return word + SYLLABLE_DELIMITER + PIG_LATIN_VOWEL_ENDING;
 } else {
 for (int i = 0; i < word.length(); i++) {
 if (isVowel(word.charAt(i))) {
 return word.substring(i) + SYLLABLE_DELIMITER + word.substring(0, i) + PIG_LATIN_ENDING;
 }
 }
 return word + SYLLABLE_DELIMITER + PIG_LATIN_ENDING;
 }
 }
 private static boolean isVowel(char c) {
 return c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u';
 }
}

Note: The methods are static because they only need the method parameters to produce the result. Also there are no "magical" constants in the code anymore (but this is rather my preferred style).

Answer 2

Regarding the following code:

// if any digits are found in str, return null
for (int i = 0; i < strArray.size(); i++) {
 char current = strArray.get(i);
 if (Character.isDigit(current)) {
 return null;
 }
}

if it were up to me, I'll probably refactor this into:

public String translate(String str) {
 Pattern digitPattern = Pattern.compile(".*\\p{Digit}.*");
 Matcher containsDigit = digitPattern.matcher(str);
 if (!containsDigit.matches()) {
 //proceed with the rest of the translation logic
 } else {
 return null;
 }
}

If we're speaking of actual efficiency (as in performance) though, I don't know how much more efficient pattern matching is compared to iteration of characters. However, this is much more readable. In any case, notice that the actual translation logic is wrapped inside the condition that the input string does not contain a number. This way, you return from the method as soon as you determine the input is unacceptable.

To check whether the input string starts with a vowel, you can also use pattern matching like:

Pattern startsWithVowelPattern = Pattern.compile("^[aeiouAEIOU].*");
Matcher startsWithVowel = startsWithVowelPatter.matcher(str);
if (startsWithVowel.matches()) {
 //do translate
}

Alternatively, you can do the following to check if the string starts with a vowel:

if (vowels.contains(str.substring(0, 1))) {
 //translate
}

• maaartinus: substring(0, 0) is the empty string, you mean substring(0, 1) or better charAt(0).
• Psycho Punch: Oh yeah, sorry about that. I've corrected the code. charAt, however, will not work because contains expect a CharSequence.