current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 8 down vote favorite

I was trying this problem and I was wondering if there is a way to check for the edge cases inside the for-loop.

The problem statement:

We'll say that a lowercase 'g' in a string is "happy" if there is another 'g' immediately to its left or right. Return true if all the g's in the given string are happy.

Here I am starting the loop from index 1 as I need to look back and I don't want an ArrayIndexOutOfBounds exception. The same applies for the end edge case where I need to look the following index.

To avoid this problem I'm checking these cases right after I've checked all the others, but it doesn't feel quite right. Is there a way to do it in a more compact way?

public boolean gHappy(String str) {

   if(str.length() == 1 && str.charAt(0) == 'g') return false; 
   if(str.length() == 0) return true;

   for(int i = 1; i < str.length() - 1; i ++) {
     if(str.charAt(i) == 'g') {
       if(str.charAt(i - 1) != 'g' && str.charAt(i + 1) != 'g') return false;     
     } 
   }
 // edge cases (start-end)
 if(str.charAt(0) == 'g' && str.charAt(1) != 'g') return false;
 if(str.charAt(str.length() - 1) == 'g' && str.charAt(str.length() - 2) != 'g') return false;

 return true;
}
share|improve this question
1  
Left OR right? What about left AND right? That count as happy? Or is the middle counted as crowded and unhappy? –  WernerCD yesterday

3 Answers 3

up vote 8 down vote accepted

There's several 'special cases' in your code.

  • String of length 1
  • String of length 0
  • 'g' at start of String
  • 'g' at end of String

I'd recommend getting rid of these special cases.

To do this, you can check for special conditions inside the for-loop.

Loop through all the positions in the string, and check if there is a 'g' on that position. If it is, then check if the position is more than 0 (i.e. it has a char in front of it) or if the position is less than str.length - 1 (i.e. it has a char after it).

The details about how to write this can be done in oh so many ways, but here is what I would end up with:

for (int i = 0; i < str.length(); i++) {
    char ch = str.charAt(i);
    if (ch == 'g') {
        if (i > 0 && str.charAt(i - 1) == ch) {
            continue;
        }
        if (i < str.length() - 1 && str.charAt(i + 1) == ch) {
            continue;
        }
        return false;
    }
}

return true;
share|improve this answer
1  
Very clean :) Nice solution –  Evan Bechtol yesterday
    
I would merge the if s into one or, of course on different lines to keep readability high. –  Caridorc yesterday
1  
@Caridorc I was considering doing that, then I realized the "can be done in oh so many ways" part. –  Simon André Forsberg yesterday
1  
You can also remove the edge cases not through logic, but by simply modifying the input, i.e. str = "." + str + "."; –  hatchet 23 hours ago
up vote 5 down vote

You can remove two if-statements altogether if you think about it! Also, you don't even need your first edge-case if you do this:

 public boolean gHappy(String str) {

   if(str.length() == 1 && str.charAt(0) == 'g') return false; 
   if(str.length() == 0) return true;

   for(int i = 1; i < str.length() - 1; i ++) {
     if(str.charAt(i) == 'g' && ((str.charAt(i - 1) != 'g' && str.charAt(i + 1) != 'g'))){
       return false;     
     } 
   }

   // edge cases (start-end)
   if(str.charAt(str.length() - 1) == 'g' && str.charAt(str.length() - 2) != 'g'){
      return false;
   }
   return true;
 }

Also, you should always use brackets in your statements, even if it's just one line. Not using brackets makes things harder for other programmers to read your code and increases the probability that unintended bugs arise.

share|improve this answer
up vote 3 down vote

The right tool for the right job:

Regular Expressions

A Regular Expressions often known as Regexp or Regex is a pattern that may or may not match a text according to some rules.

The simplest of Regexes is the literal one:

"foo".match("f")

An alphabetic character will match itself.

There are many many rules and patterns that you can use and I suggest learning them, anyway we only need this very basic and another:

[^<character(s)>]

that matches everything but the character(s).

For example [^a] matches everything but a.

It is your challange

This two tools will allow you to solve this problem using Regexp, just for future reference I include the code (in Ruby but Regexes are pretty similar between languages) that solves the problem (tested), but please think about this for yourself:

def geppy(string) not string.match("[^g]g[^g]") end

Fixing edge cases in a non-elegant manner:

def geppy(string)
  return false if string.length == 1
  return false if string.length == 2 && string != 'gg'
  return (not string.match("[^g]g[^g]"))
end

share|improve this answer
1  
It looks like your solution does not handle some edge cases like "g" and "ag" properly. –  Jabe yesterday
    
@Jabe my suuny day testing was not enough... –  Caridorc yesterday
    
@Jabe fixed now, My code passes all this tests –  Caridorc yesterday
    
What will your code return for the string ab ? (length 2) And the string a ? (Hint: They should return true, I believe...) –  Simon André Forsberg 23 hours ago
1  
Regex solution in Java: return str.matches("[^g]*(g{2,}[^g]*)*"); –  nhahtdh 15 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.