Beautiful Word: HackerRank

Question

Here is my code for hacker rank question. Can somebody suggest improvement?

Question:

We consider a word, to be beautiful if the following two conditions are satisfied:

• No two consecutive characters are the same.
• No two consecutive characters are in the following vowel set: a, e, i, o, u, y. Note that we consider y to be a vowel in this challenge.

Solution:

import java.io.*;
import java.util.*;
import java.text.*;
import java.math.*;
import java.util.regex.*;

public class Solution {

   public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        String w = in.next();

        int result = isBeautiful(w);

        if (result == 1) {
            System.out.print("Yes");
        } else
            System.out.print("No");

    }

    static int isBeautiful(String w) {
        for (int i = 0; i < w.length() - 1; ++i) {
            if (w.charAt(i) == w.charAt(i + 1)) {
                return 0;
            }
            if ( ( isVowel(w.charAt(i)) == isVowel(w.charAt(i + 1)) && (isVowel(w.charAt(i)) == 1))) {
                return 0;
            }
        }
        return 1;
    }

    static int isVowel(char c) {
        if ( (c == 'a') || (c == 'e') || (c == 'i') || (c == 'o') || (c == 'u') || (c == 'y') ) {
            return 1;
        }
        return 0;
    }

}

Answer 1

First of all, you did well splitting it up into those methods.

The major issue I have with your code is that the natural translation from the English answers yes/no to Java is to use a boolean. Where true means yes and false means no.

So your isVowel method should be written like this:

static boolean isVowel(char c) {
    if ( (c == 'a') || (c == 'e') || (c == 'i') 
        || (c == 'o') || (c == 'u') || (c == 'y') ) {
        return true;
    }
    return false;
}

Now the next thing you need to know is that a check like c == 'a' is already a boolean, and that the || operator combines 2 booleans into a new boolean. Since we want to return a boolean, instead of writing

if (<boolean check>) {
    return true; 
} else {
    return false;
}

we should just write the following:

return <boolean check>;

which in your case would be:

static boolean isVowel(char c) {
    return (c == 'a') || (c == 'e') || (c == 'i') 
           || (c == 'o') || (c == 'u') || (c == 'y');
}

A final issue with this method is that it's actually package-visible. There is no reason to use this method outside of this class, so it should be private.

private static boolean isVowel(char c) {

---

The same reasoning applies to the isBeautiful method. So let us also make this private and return a boolean.

private static boolean isBeautiful(String w) {
    for (int i = 0; i < w.length() - 1; ++i) {
        if (w.charAt(i) == w.charAt(i + 1)) {
            return false;
        }
        if (isVowel(w.charAt(i)) && isVowel(w.charAt(i + 1))) {
            return false;
        }
    }
    return true;
}

Notice how I also simplified the check for 2 consecutive vowels so that it now actually reads: if this character is a vowel and the next character is a vowel.

---

Now the last thing left to do is use this new isBeautiful method. This means we will change this code:

int result = isBeautiful(w);

if (result == 1) {
    System.out.print("Yes");
} else
    System.out.print("No");

to this:

if (isBeautiful(w) {
    System.out.print("Yes");
} else {
    System.out.print("No");
}

Notice I also added the {} after the else. It's best practice to add curly braces after each if/else/for/while.

---

One last tip is that you can also rewrite the isVowel method like so:

private static final String VOWELS = "aeiouy";

private static boolean isVowel(char c){
    return VOWELS.contains(c);
}

But this doesn't mean that it's bad to check each character separately - it's just an alternative way.

Answer 2

@Imus already posted a very nice answer, but was kind enough to leave me one thing.

You should only import the packages you really need (in this case java.util.*).

Answer 3

Do you need the second if statement at all?

if (w.charAt(i) == w.charAt(i + 1)) { return 0; } 
if ( ( isVowel(w.charAt(i)) == isVowel(w.charAt(i + 1)) && (isVowel(w.charAt(i)) == 1))) { return 0; }

The first if statement catches all instances of two consecutive characters including vowels

Answer 4

You do not need to call isVowel() on w.charAt(i) if you already tested the character with isVowel(w.charAt(i+1)) in the previous iteration. We may store the 'previous character was a vowel' in a separate variable, so we can use it in the next iteration without calling a function:

static int isBeautiful(String w) {
    boolean prevVowel = isVowel(w.charAt(0));
    boolean thisVowel = false;
    for (int i = 0; i < w.length() - 1; ++i) {
        if (w.charAt(i) == w.charAt(i + 1)) {
            return 0;
        }
        thisVowel = isVowel(w.charAt(i + 1));
        if (prevWovel && thisVowel) {
            return 0;
        }
        prevWovel = thisVowel;
    }

    return 1;
}

static boolean isVowel(char c) {
    return (c == 'a') || (c == 'e') || (c == 'i') 
           || (c == 'o') || (c == 'u') || (c == 'y');
}