Can I improve this Pig-Latin converter? [closed]_问答_开发者

Closed. This question does not meet Stack Overflow guidelines. It is not currently accepting answers.

Th开发者_StackOverflow社区is question does not appear to be about programming within the scope defined in the help center.

Closed 9 years ago.

Improve this question

I'm brand-spanking new to Java and I made this little translator for PigLatin.

package stringmanipulation;

public class PigLatinConverter {
    public String Convert(String word){
        int position = 0;
        if (!IsVowel(word.charAt(0))) {
            for (int i= 0; i < word.length(); i++) {
                if (IsVowel(word.charAt(i))) {
                    position = i;
                    break;
                }
            }
            String first = word.substring(position, word.length());
            String second = word.substring(0, position) + "ay";

            return first + second;
        } else {
            return word + "way";
        }
    }

    public boolean IsVowel(char c){
        if (c == 'a')
            return true;
        else if(c == 'e')
            return true;
        else if(c == 'i')
            return true;
        else if(c == 'o')
            return true;
        else if(c == 'u')
            return true;
        else
            return false;        
    }
}

Are there any improvements I can make?

Are there any nifty Java tricks that are in the newest Java version I might not be aware of? I come from a C# background.

Thank you!

I'd rewrite isVowel(char ch) as follows:

return "aeiou".indexOf(ch) != -1;

And I'd write the following instead:

// String first = word.substring(position, word.length());
   String first = word.substring(position);

I'd also rename method names to follow coding convention.

And of course, being me, I'd also use regex instead of substring and for loop.

System.out.println("string".replaceAll("([^aeiou]+)(.*)", "$2$1ay"));
// ingstray

References

Java Coding Convention - Naming Convention

Disclaimer: I don't know Java.

Inverted logic is confusing please write your if statement as such:

    if (IsVowel(word.charAt(0))) {
        return word + "way";
    } else {
        for (int i= 0; i < word.length(); i++) {

        // ...

        return first + second;
    }

You can even drop the else.

IsVowel may need to be private. It can also be rewritten using a single || chain, or as a "".indexOf (or whatever it is in Java).

Your for logic can be simplified int a short while:

        while (position < word.length() && !IsVowel(word.charAt(position)) {
            ++position;
        }

Here's a complete rewrite that makes the code more readable if you know how to read regex:

String[] words =
    "nix scram stupid beast dough happy question another if".split(" ");

for (String word : words) {
    System.out.printf("%s -> %s%n", word,
        ("w" + word).replaceAll(
            "w(qu|[^aeiou]+|(?<=(w)))([a-z]*)",
            "$3-$1$2ay"
        )
    );
}

This prints (as seen on ideone.com):

nix -> ix-nay
scram -> am-scray
stupid -> upid-stay
beast -> east-bay
dough -> ough-day
happy -> appy-hay
question -> estion-quay
another -> another-way
if -> if-way

Note that question becomes estion-quay, which is the correct translation according to Wikipedia article. In fact, the above words and translations are taken from the article.

The way the regex work is as follows:

First, all words are prefixed with w just in case it's needed
Then, skipping that w, look for either qu or a non-empty sequence of consonants. If neither can be found, then the actual word starts with a vowel, so grab the w using capturing lookbehind
Then just rearrange the components to get the translation

That is:

"skip" dummy w
|
w(qu|[^aeiou]+|(?<=(w)))([a-z]*)   -->  $3-$1$2ay
 \                2\_/ /\______/
  \_________1_________/    3

References

regular-expressions.info
- Character class:[…], Alternation: |, Repetition:+,*, Lookaround:(?<=…), and Capturing:(…)

I know this question is well over a year old, but I thought I would put my modification of it. There are several improvements in this code.

public String convert(String word)
{
    int position = 0;
    while(!isVowel(word.charAt(position)))
    {
        ++position;
    }
    if(position == 0)
    {
        return word + "-way";
    }
    else if(word.charAt(0) == 'q')
    {
        ++position;
    }
    return word.substring(position) + "-" + word.substring(0, position) + "ay";
}
public boolean isVowel(char character) 
{
    switch(character)
    {
        case 'a': case 'e': case 'i': case 'o': case 'u':
            return true;
        default:
            return false;
    }
}

First the code will find the position of the first vowel, and then jump out of the loop. This is simpler than using a for loop to iterate through each letter and using break; to jump out of the loop. Secondly, this will match all the testcases on the Wikipedia site. Lastly, since chars are actually a limited range int, a switch statement can be used to improve performance and readability.

Not strictly an improvement as such, but Java convention dictates that methods should start with a lowercase letter.

I'm years removed from Java, but overall, your code looks fine. If you wanted to be nitpicky, here are some comments:

Add comments. It doesn't have to follow the Javadoc specification, but at least explicitly describe the accepted argument and the expected return value and perhaps give some hint as to how it works (behaving differently depending on whether the first character is a vowel)
You might want to catch IndexOutOfBoundsException, which I think might happen if you pass it a zero length string.
Method names should be lower case.
IsVowel can be rewritten return c == 'a' || c == 'e' and so on. Due to short-circuiting, the performance in terms of number of comparisons should be similar.

Is this homework? If so, tag it as such.

Unclear what expected behaviour is for "honest" or "ytterbium".
It doesn't respect capitals ("Foo" should turn into "Oofay", and AEIOU are also vowels).
It crashes if you pass in the empty string.