What does the Bob say?

Description:


Bob is a lackadaisical teenager. In conversation, his responses are
very limited.


Bob answers 'Sure.' if you ask him a question.


He answers 'Whoa, chill out!' if you yell at him.


He says 'Fine. Be that way!' if you address him without actually
saying anything.


He answers 'Whatever.' to anything else.

Code:

import java.util.Optional;

enum SentenceType {
  Question, Yell, Silence, Misc
}

public class Bob {
  public String hey(String sentence) {
    String trimmedSentence = Optional.ofNullable(sentence).orElse("").trim();
    SentenceType classifiedSentence = classify(trimmedSentence);

    if (classifiedSentence == SentenceType.Silence) {
      return "Fine. Be that way!";
    } else if (classifiedSentence == SentenceType.Question) {
      return "Sure.";
    } else if (classifiedSentence == SentenceType.Yell) {
      return "Whoa, chill out!";
    }
    return "Whatever.";

  }

  private SentenceType classify(String sentence) {
    if (sentence.isEmpty()) {
      return SentenceType.Silence;
    } else if (isAllLettersUpperCase(sentence)) {
      return SentenceType.Yell;
    } else if (isQuestion(sentence)) {
      return SentenceType.Question;
    }
    return SentenceType.Misc;
  }

  private boolean isAllLettersUpperCase(String sentence) {
    char[] charArray = sentence.toCharArray();
    boolean hasLetters = false;
    for (char ch : charArray) {
      if (Character.isLetter(ch)) {
        hasLetters = true;
        if (Character.isLowerCase(ch)) {
          return false;
        }
      }
    }
    return hasLetters && true;
  }

  private boolean isQuestion(String sentence) {
    return (sentence.charAt(sentence.length() - 1) == '?') ? true : false;
  }
}

Test Suite:

```
import org.junit.Test;

import static org.junit.Assert.*;

public class BobTest {
private final Bob bob = new Bob();

@Test
public void saySomething() {
assertEquals(
"Whatever.",
bob.hey("

Avoid if chains with enumerations

You are using an enumeration for all the possible responses of Bob. This is a great idea, and you can take it a step further. Currently, you have:

SentenceType classifiedSentence = classify(trimmedSentence);

if (classifiedSentence == SentenceType.Silence) {
    return "Fine. Be that way!";
} else if (classifiedSentence == SentenceType.Question) {
    return "Sure.";
} else if (classifiedSentence == SentenceType.Yell) {
    return "Whoa, chill out!";
}
return "Whatever.";

which consists of multiple if statement in order to return the correct output.

Consider making the sentence a property of the enum:

enum SentenceType {
    Question("Sure."),
    Yell("Whoa, chill out!"),
    Silence("Fine. Be that way!"),
    Misc("Whatever.");

    private final String sentence;

    SentenceType(String sentence) {
        this.sentence = sentence;
    }

    public String getSentence() {
        return sentence;
    }
}

This way, you can refactor this block:

SentenceType classifiedSentence = classify(trimmedSentence);
return classifiedSentence.getSentence();

without any ifs.

Regarding your concern of the order of the methods, I don't see it as an issue: it depends on the order only because you decided you write your conditions as early-return. This means that the check to decide if Bob is yelling is pre-supposing that Bob actually said something, which also means that you don't explicitely check that Bob said something to determine if he's yelling. This indeed couples the order but it does simplify the checks involved: otherwise, you would need to add a !sentence.isEmpty() && ... to every condition.

Optional

You are using an Optional for the following:

String trimmedSentence = Optional.ofNullable(sentence).orElse("").trim();

This is over-kill, it would be simpler (and perhaps clearer) to just use a ternary operator:

String trimmedSentence = sentence == null ? "" : sentence.trim();

Stream API

You could refactor your isAllLettersUpperCase method to the following

private boolean isAllLettersUpperCase(String sentence) {
    return sentence.chars().anyMatch(Character::isLetter) && 
            sentence.chars().filter(Character::isLetter).allMatch(Character::isUpperCase);
}

which may be easier to read: it returns true if there at least one letter and all letters are uppercase.

StackExchange Code Review Q#127318, answer score: 25