6
\$\begingroup\$

This is web exercise 3.1.45. from the book Computer Science An Interdisciplinary Approach by Sedgewick & Wayne:

Write a program that reads in a list of words from the command line and prints true if they form a word chain and false otherwise. In a word chain, adjacent words must differ in exactly one letter, e.g., HEAL, HEAD, DEAD, DEED, DEER, BEER.

Here is my program:

public class WordChainChecker 
{
    public static boolean checkWordCouple(String word1, String word2)
    {
        int counter = 0;
        int wordLength = word1.length();
        for (int i = 0; i < wordLength; i++)
        {
            if (word1.charAt(i) != word2.charAt(i)) counter++;
        }
        if (counter > 1) return false;
        else             return true;
    }
    public static void main(String[] args)
    {
        String word1 = StdIn.readString();
        String word2 = StdIn.readString();
        boolean truth = true && checkWordCouple(word1,word2);
        while (!StdIn.isEmpty())
        {
            word1 = StdIn.readString();
            truth = truth && checkWordCouple(word1,word2);
            word2 = word1; 
        }
        System.out.println(truth);
    }
}

StdIn is a simple API written by the authors of the book. I checked my program and it works.

Is there any way that I can improve my program?

Thanks for your attention.

\$\endgroup\$
10
\$\begingroup\$

Requirements

Number of words

Are you guaranteed to have at least 2 words? If so, you could simplify your loop, by removing the first read; the loop will take care of the second & subsequent words:

    String word2 = StdIn.readString();
    boolean truth = true;
    while (!StdIn.isEmpty())
    {
        String word1 = StdIn.readString();
        truth = truth && checkWordCouple(word1,word2);
        word2 = word1; 
    }

Although now if given a single word, it will produce true. This may be an improvement, but you’d need to check the exact requirements to be sure.

Word Length

Is HEAD, HEAL, TEA, TO a word chain? Your program says it is!

What about HEAD, HEAL, HEALTHY? That input causes your program to crash!

Your program should handle mismatched word length, unless the problem statement guarantees all inputs will be the same length.

Number of Differences

The problem description says:

In a word chain, adjacent words must differ in exactly one letter

but HEAD, HEAD, HEAD, HEAD is reported as "true". Clearly, counter > 1 is not the correct "false" condition. Borrowing Doi9t's suggestion, and further improving it, the correct line would read:

return counter == 1;

Self Documenting Code

You read word1, then word2, and compare the first to the second (let’s call that “forward”), then read a third into word1, and compare the third to the second (comparison in the “reverse” direction). You are testing for one difference, so it works, but what if you were checking for one additional character? By reversing the word order, you’d actually be testing for character removal!

Instead of word1 & word2, maybe previous_word and next_word would help keep things clearer.

    String previous_word = StdIn.readString();
    boolean truth = true;
    while (!StdIn.isEmpty())
    {
        String next_word = StdIn.readString();
        truth = truth && checkWordCouple(previous_word, next_word);
        previous_word = next_word; 
    }
\$\endgroup\$
5
  • \$\begingroup\$ I checked my program with "Head Tea To" and "Head Heal Healthy" and it produces error. Could you please explain more? Do you mean that I should add exception handling to produce meaningful errors? \$\endgroup\$ Sep 8 '20 at 9:38
  • 1
    \$\begingroup\$ Sorry. "HEAD HEAL TEA TO" will incorrectly produce "true" not crash; forgot the first two words are compared in a different order from the rest. No — I wouldn't add exception handling. But I would add do something differently in checkWordCouple. You get the length of word1, and then blindly assume word2 is the same length. Perhaps you could check that? Would it make sense to return false if they don't match? \$\endgroup\$
    – AJNeufeld
    Sep 8 '20 at 13:51
  • \$\begingroup\$ I think so, if word1.length() != word2.length() it should return false. \$\endgroup\$ Sep 8 '20 at 13:58
  • \$\begingroup\$ Great review, one question. Any reason why suggesting to use previous_word instead of previousWord? Given that OP is already using camel case. \$\endgroup\$
    – Marc
    Sep 9 '20 at 13:16
  • \$\begingroup\$ @Marc Certainly: I'm posting from an iPhone, and keeping track of everything in my head while posting is hard. Ok, perhaps not the best of reasons... \$\endgroup\$
    – AJNeufeld
    Sep 9 '20 at 13:30
6
\$\begingroup\$

I have some suggestions for your code.

Simplify the boolean conditions.

Generally, when you are returning both true and false surrounded by a condition, you know you can refactor the logic of the expression.

if (counter > 1) return false;
else             return true;

can be changed into

return counter <= 1;

By changing the range, we can make the condition shorter.


boolean truth = true && checkWordCouple(word1,word2);

can be changed into

boolean truth = checkWordCouple(word1,word2);

The and operator is useless (true && false == false and true && true == true) and is equivalent to a single boolean given by the checkWordCouple method.

Use the simplified operator when possible.

EDIT: As stated in the comments of this answer, the use of the Bitwise AND assignment operator is not recommended in this case.

Java gives you plenty of operators to do multiple operations.

In your case, you can use the Bitwise AND assignment operator (&=) instead of reassigning the boolean each time with the result of ruth && checkWordCouple(word1,word2). java truth = truth && checkWordCouple(word1,word2);

into

java truth &= checkWordCouple(word1,word2);

This operator will assign itself the result of the previous value && current.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ I agree with "simplifying the boolean conditions", but replacing the logical and && with the bitwise and assignment &= sounds like a mistake. \$\endgroup\$
    – Stef
    Sep 8 '20 at 12:39
  • \$\begingroup\$ The bitwise will only be false when the checkWordCouple method return false and the value will be kept until the end of the loop (any false value found in the loop, and the final condition will stay false until the end). I don’t think this is a mistake, since both of the results will be similar. truth = truth && checkWordCouple(word1,word2) == truth &= checkWordCouple(word1,word2); 15.22.2. Boolean Logical Operators \$\endgroup\$
    – Doi9t
    Sep 8 '20 at 15:04
  • 2
    \$\begingroup\$ @Stef Both the bitwise-and (&) and logical-and (&&) can be used with boolean values in Java. However, when bitwise-and is used, it does not perform short-circuit evaluation. false && checkWordCouple(word1, word2) will never call checkWordCouple(), where as false & checkWordCouple(word1, word) will always call it, as will truth &= checkWordCouple(word1, word2);, so && should be preferred if the call has no side-effects to optimize performance. \$\endgroup\$
    – AJNeufeld
    Sep 8 '20 at 16:39
  • \$\begingroup\$ Totally forgot about the short-circuit, my bad. \$\endgroup\$
    – Doi9t
    Sep 8 '20 at 16:42
  • \$\begingroup\$ Other than playing Code-Golf, using the binary operators with boolean values doesn't have much value ... with the exception of xor. There is no logical-xor operator (^^), but the binary-xor operator (^) does work with boolean. There is no short-circuit evaluation possible for xor, so making the single character, non-short-circuiting ^ operator work for boolean values, is perhaps the justification for also allowing & and | to work. \$\endgroup\$
    – AJNeufeld
    Sep 8 '20 at 16:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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