Beautiful Word: HackerRank

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;
}

}

• this might solve the problem but it isn't very OOP, and looks difficult to interpret and not very amenable to change. Looks like an interesting problem though. i might have a go shortly Apr 14, 2017 at 5:04

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.

• nice answer - I'll go ahead and cancel mine ;-) Apr 11, 2017 at 17:48

@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.*).

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

• But it does not catch every two consecutive vowels – for example it catches ee or oo but not ae or yi. Apr 14, 2017 at 11:21
• You are right, i miss read the question 😀 Apr 14, 2017 at 16:17

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 (prevVowel && thisVowel) {
return 0;
}
prevVowel = thisVowel;
}

return 1;
}

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