Vowels ad infinitum...
str.length() <= 3 && (str.charAt(i) == 'a' || str.charAt(i) == 'e' || str.charAt(i) == 'i' || str.charAt(i) == 'o' || str.charAt(i) == 'u')
could be simpler and clearer:
public static final String VOWELS = "aeiou";
// ...
str.length() <= 3 && VOWELS.indexOf(str.charAt(i)) != -1
Then you have a repeated check for vowels again in the else if
clause of the conditional that originally check for vowels. To avoid repeated checks, just wrap the entire block in an if (VOWELS.indexOf(str.charAt(i)) != -1)
This is generalizeable: any time there are two parts of the same conditional that do the same check, it's probably better to wrap the entire thing in that one check.
And every other time you encounter (str.charAt(i) == 'a' || str.charAt(i) == 'e' || str.charAt(i) == 'i' || str.charAt(i) == 'o' || str.charAt(i) == 'u')
, it should be replaced by VOWELS.indexOf(str.charAt(i)) != -1
But make sure it's exactly identical. (Note that there are more efficient ways to do this, but it gets complicated quickly, and my familiarity with Java is limited.)
Loopy code smells
// ...
i = 1;
while (str.charAt(i) != 'a' || str.charAt(i) != 'e' || str.charAt(i) != 'i' || str.charAt(i) != 'o' || str.charAt(i) != 'u') {
i++;
//...
Seeing this gives me pause: having an iteration variable in a while loop is bad style. There's a reason the for loop
exists!
There's another, even more glaring problem though. This loop condition will never fail on its own.
One can figure that out simply from the first two conditions (str.charAt(i) != 'a' || str.charAt(i) != 'e'
): even if the character is an 'a'
, it cannot also be an 'e'
; if it's not an 'a'
, then it's true on the first condition. Replacing it with while (true)
would do the same thing.
Unreachable statement
Examine this:
if (firstVow != secondVow) {
//...
break;
} else if (firstVow == secondVow) {
//...
break;
}
scan.close();
One may note that because firstVow
either equals or does not equal SecondVow
at any given point, this last statement will never be reached and the scanner will never be closed. Bummer. (It actually will later, and this statement can be safely ignored and deleted.) This is a good reason to separate the scanning from the translating method; it makes it easier to change both (e.g. add validation, etc.), and it prevent resource leaks.
More transparent code
Hopefully more transparent code can help you identify the source of the bugs.
package pigLatin;
import java.util.Scanner;
public class PigLatin
{
public static final String VOWELS = "aeiou";
public static String translate(String str)
{
pigLatinPrefix = "";
char firstVow;
if (VOWELS.indexOf(str.charAt(0)) != -1) {
if (str.length() > 3) {
firstVowel = str.charAt(0);
for (int i = 1; i < str.length(); ++i) {
if (VOWELS.indexOf(str.charAt(i)) != -1) {
char secondVowel = str.charAt(i);
//issues arise if the first vowel is equal to the second one
if (firstVowel != secondVow) {
pigLatinPrefix = str.substring(str.indexOf(secondVow), str.length()) + str.substring(str.indexOf(firstVow), str.indexOf(secondVow));
break;
} else {
pigLatinPrefix = str.substring(2) + firstVowel + str.charAt(1);
break;
}
}
}
}
}
//checks for words that begin with consonants
else {
for (int i = 1; i < str.length(); ++i) {
if (VOWELS.indexOf(str.charAt(i)) != -1) {
firstVowel = str.charAt(i);
pigLatinPrefix = str.substring(str.indexOf(firstVow), str.length()) + str.substring(0, str.indexOf(firstVow));
break;
}
}
}
if (pigLatinPrefix.equals("")) {
pigLatinPrefix = str
}
return pigLatinPrefix + "ay";
}
public static void main(String[] args) {
Scanner scan = new Scanner(System.in);
str = scan.next();
scan.close();
System.out.println(translate(str));
}
}
Stylistic concerns
To begin with, please do not indent your code only 1 space! Studies have shown that code is most comprehensible when indented 2-4 spaces. My personal preference, along with many other programmers, is 4 spaces.1
A consistent bracketing style wouldn't hurt either.
1: Naturally, the first thing I did when examining your code was apply the regex \r\n( +)
-> \r\n\1\1\1\1