Parsing speech out of text

Question

I'm working on an experiment to parse speech out of an emote in a text roleplaying game (MUD) so that if you can't hear it correctly, or if it's in a different language, you won't understand it. This works correctly, but I was just wanting to make sure it was as clear and efficient as possible.

import java.util.Scanner;
import java.util.Random;

public class EmoteParser {
    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);

        System.out.print("Enter an emote: ");
        String emote = scanner.nextLine();

        System.out.println("Original emote:\n" + emote);
        System.out.println("\nParsed emote:\n" + jumbleEmoteSpeech(emote));
    }

    public static String jumbleEmoteSpeech(String emote) {
        StringBuilder out = new StringBuilder();
        int quote1Location = emote.indexOf('"');

        if (quote1Location == -1) {
            out.append(emote);
        }
        else {
            int quote2Location;
            int currentIndex = 0;
            boolean found = true;
            do {
                quote2Location = emote.indexOf('"', quote1Location + 1);
                if (quote2Location == -1 || quote2Location < currentIndex) {
                    out.append(emote.substring(currentIndex));
                    found = false;
                }
                else {
                    out.append(emote.substring(currentIndex, quote1Location + 1));
                    out.append(jumble(emote.substring(quote1Location + 1, quote2Location)));
                    currentIndex = quote2Location;

                    quote1Location = emote.indexOf('"', currentIndex + 1);
                }
            } while (found);
        }

        return out.toString();
    }

    public static String jumble(String text) {
        StringBuilder out = new StringBuilder();
        Random generator = new Random();

        for (int i = 0; i < text.length(); i++) {
            char ch = text.charAt(i);
            if (Character.isLetter(ch)) {
                if (Character.isUpperCase(ch)) {
                    out.append((char)('A' + generator.nextInt(26)));
                }
                else {
                    out.append((char)('a' + generator.nextInt(26)));
                }
            }
            else if (Character.isDigit(ch)) {
                out.append((char)('0' + generator.nextInt(10)));
            }
            else {
                out.append(text.charAt(i));
            }
        }

        return out.toString();
    }
}

Also, is there any real benefit to using '"' vs "\""? I think '"' is more readable, and any performance difference is probably optimized out anyway.

Answer 1

The jumble method

In the jumble method, you are looping over the character by having an index going from 0 to the length of the input array.

for (int i = 0; i < text.length(); i++) {
    char ch = text.charAt(i);
    // ...
}

Since you don't use the index apart from retrieve the character, you couls just use toCharArray() and loop directly over the characters:

for (char ch : text.toCharArray()) {
    // ...
}

Then, you are appending characters based on their codepoint. For this, you are doing

out.append((char) ('A' + generator.nextInt(26))); // same for 'a' and '0'

Instead of having this (ugly) cast, you could directly use appendCodePoint with

out.appendCodePoint('A' + generator.nextInt(26));

Additionally, the code path to jumble a character can lead to potential bugs:

if (Character.isLetter(ch)) {
    if (Character.isUpperCase(ch)) {
        // ...
    } else {
        // ...
    }
} else if (Character.isDigit(ch)) {
    // ...
} else {
    // ...
}

The problem is that isLetter returns true for characters that are not only uppercase or lowercase characters. For example, it would return true for all those weird characters classified as "Lo" as per Unicode (and OTHER_LETTER in Java). It would also return true for those classified "Lm" (MODIFIER_LETTER in Java), like "OL CHIKI AHAD" ᱽ. Since I think that, for example, in the case of the Hebrew letter "BET", ב, you don't want to consider it lowercase (which your code would, since it is a letter not uppercase), you can remove that check completely and have:

if (Character.isUpperCase(ch)) {
    sb.appendCodePoint('A' + generator.nextInt(26));
} else if (Character.isLowerCase(ch)) {
    sb.appendCodePoint('a' + generator.nextInt(26));
} else if (Character.isDigit(ch)) {
    sb.appendCodePoint('0' + generator.nextInt(10));
} else {
    sb.append(ch);
}

This way, only character that have case (upper or lower) will be taken into account in those branches. But still, there are a lot of lowercase letters (and uppercase letters) different than a-z, like ῲ or ⅎ or even ⱙ (which is apparently "GLAGOLITIC SMALL LETTER IOTATED BIG YUS"). In those case, they will be replaced by a Latin small character, which may or may not be what you want. If you truly want to jumble only Latin letters, it would be best to test the character against 'a' and 'z' (conversely for uppercase). And you know what? This is not just letters. There are a lot of digits that aren't 0-9, like ٨ (Arabic 8) or ᧗ (New Tai Lue 7) or even ꧓ (which is apparently Javanese 3). Therefore, if you just want to concentrate on Latin letters and Roman digits, you can just use

if (ch >= 'A' && ch <= 'Z') {
    sb.appendCodePoint('A' + generator.nextInt(26));
} else if (ch >= 'a' && ch <= 'z') {
    sb.appendCodePoint('a' + generator.nextInt(26));
} else if (ch >= '0' && ch <= '9') {
    sb.appendCodePoint('0' + generator.nextInt(10));
} else {
    sb.append(ch);
}

The last concern is about the Random object. You are creating one each time this method is executed. It would be preferable to create only one and reuse the same everytime. For example, you can have a constant

private static final Random RANDOM = new Random();

The jumbleEmoteSpeech method

You should always prefer to return early from methods when it is possible. In this case, if there are no quotes in the String, you return it unchanged.

StringBuilder out = new StringBuilder();
int quote1Location = emote.indexOf('"');
if (quote1Location == -1) {
    out.append(emote);
} else {
    // ...
}
return out.toString();

You could write this more concisely with

int quote1Location = emote.indexOf('"');
if (quote1Location == -1) {
    return emote;
}
// ...

This accomplishes two things:

• There is no StringBuilder created: we don't need one
• The code is more readable because the else part isn't needed anymore: it shifts the rest with one indentation less.

Also, prefer to declare variables at the moment of use. In this case, you are declaring int quote2Location; before the while loop:

int quote2Location;
do {
    quote2Location = emote.indexOf('"', quote1Location + 1);
    // ...
} while (...);

Instead, you can just have:

do {
    int quote2Location = emote.indexOf('"', quote1Location + 1);
    // ...
} while (...);