Pig Latin translator: Java says dead code

Question

Java is complaining about dead code in my translator:

public static void pigLatinify(String fname) throws IOException 
{
    File file = new File("projectdata.txt");

    try 
    {
        Scanner scan1 = new Scanner(file);
        while (scan1.hasNextLine()) 
        {
            Scanner scan2 = new Scanner(scan1.nextLine());
            while (scan2.hasNext())
            {
                String s = scan2.next();
                boolean b = Syllable.countSyllables(s);
                char firstLetter = s.charAt(0);
                if (b = false && firstLetter=='a' || firstLetter=='i' || firstLetter=='o' || firstLetter=='e' || 
                        firstLetter=='u' || firstLetter=='A' || firstLetter=='I' || firstLetter=='O' || 
                        firstLetter=='E' || firstLetter=='U')
                {
                    String output = s + "hay" + " ";
                    System.out.print(output);
                }
                else if (b = true && (firstLetter=='a' || firstLetter=='i' || firstLetter=='o' || firstLetter=='e' || 
                        firstLetter=='u' || firstLetter=='A' || firstLetter=='I' || firstLetter=='O' || 
                        firstLetter=='E' || firstLetter=='U')) 
                {
                    String output = s + "way" + " ";
                    System.out.print(output);
                }
                else 
                {
                    String restOfWord = s.substring(1);
                    String output = restOfWord + firstLetter + "ay" + " ";
                    System.out.print(output);
                }

            }
            System.out.println("");
            scan2.close();
        }
        scan1.close();
    } 

    catch (FileNotFoundException e) 
    {
        e.printStackTrace();
    }
}

The method called (Syllable.countSyllables) is:

class Syllable
{
    public static boolean countSyllables(String word)
    {
        int vowelCount = 0;
        word.toLowerCase();
        for (int a = 0; a < word.length(); a++)
        {
            char test = word.charAt(a);
            if (test == 'a' || test == 'e' || test == 'i' || test == 'o' || test == 'u')
            vowelCount++;
        }
        if (vowelCount > 1)
            return true;
        return false;
    }
}

Java says the result of local variable boolean b isn't being used. It also flags up the first "if" statement and says that the "firstLetter=='a' is dead code. It doesn't flag anything else up though.

Have I done something very silly I'm missing??

Thanks!

Answer 1

I don't know Java much, but I see a possible syntax error.

if (b = false && firstLetter=='a' || firstLetter=='i' || firstLetter=='o' || firstLetter=='e' || 
                    firstLetter=='u' || firstLetter=='A' || firstLetter=='I' || firstLetter=='O' || 
                    firstLetter=='E' || firstLetter=='U')

Change to: b=false to b==false

Looks like you're setting the boolean value, not checking what it equals. It could be that the countSyllables is not actually being used (thus dead) since you're only turning around to set b to something else. Technically only true, so it's also not being used.

Disclaimer: Not a Java programmer, just C#.

Answer 2

I'm going to go beyond the scope of the original question to do a more thorough critique. Please don't be overwhelmed — you can take as much or as little of this advice as you wish.

The main faults I see are:

• Separation of concerns. Your has to do multiple things, and it tries to do them all together:

  • File input
  • Tokenizing
  • Pig Latin transformation
  • Output

  If you separate these problems into layers, your code will be easier to read, maintain, and reuse.

• Complicated conditions. Your if-conditions are complex, combining many boolean expressions with && and ||. Furthermore, similar conditions appear several times. Both of these problems are Bad Code Smells, and are contributing factors to the error that led you to ask this question in the first place.

  What you wanted to do was detect vowels. There are more expressive ways to handle strings than to work character by character.

• Excessive instantiations of Scanners. You should be able to handle the whole file using just one Scanner.

• FileNotFoundException got swallowed. Since the pigLatinify() method throws IOException anyway, you're probably better off letting FileNotFoundException propagate to the caller.

• pigLatinify(String) interface is prone to misuse. If the method expects a filename, then it should probably take a File argument rather than a String. Otherwise, someone might get the impression that you should pass the textual data to be transformed rather than the name of the file containing the data.

Putting everything together...

/**
 * Regular expression to match and capture a word and capture any
 * inter-word characters that follow the word.  A word consists of
 * Unicode letters and may contain one apostrophe.
 */
private static Pattern wordAndJunkPat = Pattern.compile("((?:\\p{L}+(?:'\\p{L}+)?)?)([^\\p{L}]*)");

public static void pigLatinify(File f) throws IOException {
    Reader r = new InputStreamReader(new FileInputStream(f));
    Writer w = new OutputStreamWriter(System.out);
    pigLatinify(r, w);
}

public static void pigLatinify(Reader r, Writer w) throws IOException {
    Scanner scan = new Scanner(r);
    try {
        while (true) {
            while (null != scan.findInLine(wordAndJunkPat)) {
                MatchResult result = scan.match();
                w.write(pigLatinifyWord(result.group(1)));
                w.write(result.group(2));
            }
            w.write("\n");
            w.flush();
            scan.nextLine();
        }
    } catch (NoSuchElementException eof) {
        scan.close();
    }
}

public static String pigLatinifyWord(String word) {
    if ("".equals(word)) {
        return "";
    }
    char firstChar = word.charAt(0);
    if ("AaEeIiOoUu".indexOf(firstChar) >= 0) {
        // word starts with vowel
        return word + (Syllabifier.isMultiSyllabic(word) ? "way" : "hay");
    } else {
        // word starts with consonant
        return word.substring(1) + firstChar + "ay";
    }
}

As for the syllable counter:

• countSyllables() is poorly named. The name suggests that it should return the number of syllables, not a boolean.

  public class Syllabifier {
  
      private static Pattern multiVowel = Pattern.compile("[aeiou].*[aeiou]", Pattern.CASE_INSENSITIVE);
  
      /**
       * Determines whether a word contains multiple syllables.
       * TODO: The current heuristic is overly simplistic: it just checks
       * whether the word contains multiple vowels.  For a better
       * heuristic, see http://stackoverflow.com/questions/405161
       */
      public static boolean isMultiSyllabic(String word) {
          return multiVowel.matcher(word).find();
      }
  }
  

Answer 3

few things about the code.

The if-else block seems to be clumsy. you could try something like-

boolean b = Syllable.countSyllables(s);
char firstLetter = s.charAt(0);
String output = "";
Collection<Character> letters = Arrays.asList('a', 'e', 'i', 'o', 'u',
                                              'A', 'E', 'I', 'O', 'U');
if (letters.contains(firstLetter)) {
  output = b ? (s + "hay" + " ") : (s + "way" + " "); 
} else {
  // do something
}
System.out.println(output);
// rest of the code...

Answer 4

Just going to add some pointers (some style, some functional):

• It's unlikely in this small program, but if your scanners fail while translating, they will not be closed properly. I would take a look at how you can better handle them in the case of failure (finally block or Java 7's try-with-resources). Proper resource handling is a good habit to learn early!
• A FileNotFoundException can be avoided here

• The result of word.toLowerCase() isn't stored (see below), so it isn't going to work as intended.

• Your countSyllables(String) method probably isn't named right for its purpose. Something like hasMultipleSyllables(String) might be better. Since you are only looking for a vowelCount > 1, you can return earlier. Perhaps something like:

  public static boolean hasMultipleSyllables(String word)
  {
      String w = word.toLowerCase(); // Don't forget to store this method result!
      int vowelCount = 0;
      for (int i = 0; i < w.length(); i++)
      {
          if (isVowel(w.charAt(i))
          {
              vowelCount++;
              if (vowelCount > 1) 
                  return true;
          }
      }
      return false;
  }
  
  private static boolean isVowel(char c)
  {
      return (c == 'a' || c == 'e' || c == 'i' || c == 'o' || c == 'u')
  }
  

This is out of scope for this project/program, but it wouldn't hurt to look over some of the widely used libraries out there for some help/ideas/inspiration in the future!

Apache Commons Lang

Guava Strings

Answer 5

A side point, and probably out of scope for this exercise, but your method for counting syllables is going to fail in many cases. Many one-syllable words have multiple vowels (e.g. beef, here, each, or my favorite: queue). You could optimize it by looking for vowels separated by a consonant, which would be more accurate, but still not perfect. (e.g. lion has the same format as beef, but would generally be considered to have 2 syllables.) You could get more specific, ignoring the letter 'e' at the end of a word, or the 'u' after a 'q', and counting certain vowel-pairs as a single vowel. Unfortunately, there are so many irregularities in the English language that it's far from trivial to program a computer to recognize how many syllables there are. Though these rules should catch most of them.