Morse code string

Question

I was bored recently and decided to write a class, MorseString. What this class does, is it takes a String that is coded like Morse (.... ..) and stores it. It also translates the String that is coded like Morse into a more "English-friendly" way (HI). This is just the basics I have written; there is much more to come.

public class MorseString {

    public static final char CHAR_SEPARATOR = ' ';
    public static final char WORD_SEPARATOR = '/';
    public static final char DOT = '.';
    public static final char DASH = '-';

    private String string;
    private String codeString;

    /*
     * Constructor that takes the Morse Code as a String as a parameter
     */

    public MorseString(String s) {
        if(!isValidMorse(s)) {
            throw new IllegalArgumentException("s is not a valid Morse Code");
        }
        if(!s.isEmpty()) {
            this.string = translate(s);
        } else {
            this.string = s; 
        }
        this.codeString = s;
    }

    /*
     * Checks if it is a valid Morse Code
     */

    private boolean isValidMorse(String s) {
        return s.matches("[" + DOT + "\\" + DASH + WORD_SEPARATOR + "\\s" + "]*");
    }

    /*
     * Traslates from Morse in a String to a String
     * e.g. ".... .." to "hi"
     */

    private String translate(String code) {
        String[] words = code.split(Character.toString(WORD_SEPARATOR));
        StringBuilder result = new StringBuilder(words.length * words[0].length()); // Rough guess of size
        for(String word : words) {
            String[] letters = word.trim().split(Character.toString(CHAR_SEPARATOR));
            for(String letter : letters) {
                result.append(MorseCode.decode(letter));
            }
            result.append(CHAR_SEPARATOR);
        }
        return result.toString().substring(0, result.length() - 1);
    }

    public static MorseString parse(String s) {
        if (!s.matches("[\\s\\dA-Za-z]*")) {
            throw new IllegalArgumentException("String too complicated");
        } else if(s.isEmpty()) {
            return new MorseString("");
        }
        int length = s.length();
        StringBuilder result = new StringBuilder(length * 4); // Rough estimate of length
        for(int i = 0; i < length; i++) {
            if(s.charAt(i) == ' ') {
                result.append(WORD_SEPARATOR).append(CHAR_SEPARATOR);
                continue;
            }
            result.append(MorseCode.encode(s.charAt(i))).append(CHAR_SEPARATOR);
        }
        return new MorseString(result.toString().trim());
    }

    /*
     * Returns the code as a String
     * e.g. if the object represents "hi" in Morse, it returns ".... .."
     */

    @Override
    public String toString() {
        return codeString;
    }

    /*
     * Returns the result of the translations
     * e.g. if the object represents "hi" in Morse, it returns "hi"
     */

    public String asString() {
        return string;
    }

}

enum MorseCode {

    A(".-"),
    B("-..."),
    C("-.-."),
    D("-.."),
    E("."),
    F("..-."),
    G("--."),
    H("...."),
    I(".."),
    J(".---"),
    K("-.-"),
    L(".-.."),
    M("--"),
    N("-."),
    O("---"),
    P(".--."),
    Q("--.-"),
    R(".-."),
    S("..."),
    T("-"),
    U("..-"),
    V("...-"),
    W(".--"),
    X("-..-"),
    Y("-.--"),
    Z("--.."),
    ZERO('0', "-----"),
    ONE('1', ".----"),
    TWO('2', "..---"),
    THREE('3', "...--"),
    FOUR('4', "....-"),
    FIVE('5', "....."),
    SIX('6', "-...."),
    SEVEN('7', "--..."),
    EIGHT('8', "---.."),
    NINE('9', "----.");

    private char character;
    private String code;

    private MorseCode(char character, String code) {
        this.character = character;
        this.code = code;
    }

    private MorseCode(String code) {
        this.character = this.name().charAt(0);
        this.code = code;
    }

    public static char decode(String s) {
        for(MorseCode mCode : MorseCode.values()) {
            if(mCode.code.equals(s)) {
                return mCode.character;
            }
        }
        throw new IllegalArgumentException("s is not a valid Morse Code");
    }

    public static String encode(char c) {
        for(MorseCode mCode : MorseCode.values()) {
            if(mCode.character == Character.toUpperCase(c)) {
                return mCode.code;
            }
        }
        throw new IllegalArgumentException(c + " cannot be found");
    }

}

How to use:

MorseString string = MorseString.parse("testing this");
System.out.println(string);
System.out.println(string.asString());

Output of above:

- . ... - .. -. --. / - .... .. ...
TESTING THIS

Questions:

1. Are there any bad practices in this code?
2. The current parse() method only allows spaces, digits, and letters. Is there a way to change that?

Answer 1

In isValidMorse, you're escaping - in the regex because you have to, but you've abstracted "-" into DASH, This means if someone decides to change DASH or DOT, the escape isn't quite right any more.

I'd suggest using a pre-compiled pattern:

private static final Pattern VALID_MORSE_PATTERN = Pattern.compile(
                    "(" + Pattern.quote(DASH) 
                    + "|" + Pattern.quote(DOT) 
                    + "|" + Pattern.quote(WORD_SEPARATOR)
                    + "|\\s)*");

public static boolean isValidMorse(CharSequence ch) {
    return VALID_MORSE_PATTERN.matcher(ch).matches();
}

Notice I used CharSequence which is slightly more flexible than using String, you can do that elsewhere for String parameters. I also made isValidMorse public, so users of your class can test beforehand instead of catching an exception.

I really like the use of enums that you've done, its very much how I'd have approached it.

I would also consider changing the "for" loop in MorseCode.encode and MorseCode.decode into a Map lookup. It'd be easy enough to have a static initializer go through each value, and put into a Map<String, MorseCode> and Map<Character, MorseCode>

Perhaps you can add WORD_SEPARATOR as a MorseCode, I don't really see how its use is different than any other character.

Answer 2

I'm not a big fan of how the MorseString class is used.

MorseString string = MorseString.parse("testing this");
System.out.println(string);
System.out.println(string.asString());

Just printing the string returns the morse, but asString returns the normal text? This is pretty confusing and someone will have to dig into the code to see what prints which version.

I don't think someone will have to use both the toString and asString methods, since they already have access to the original string/morse. So why not make the whole thing static and have two public utility methods:

public static String ToMorse(String text);
public static String ToText(String morse);

Then it will be much more clear how to use it.

Answer 3

Use the ternary operator(?) to simplify this:

if(!s.isEmpty()) {
    this.string = translate(s);
} else {
    this.string = s; 
}

To this:

this.string = s.isEmpty() ? s : translate(s);

It doesn't just improve efficiency, but also reduces typing.

Answer 4

I agree with @styxtwo: you don't need to create a MorseString constructor. If you use Java 8, you can do it like this:

public static MorseString parse(String s) {
    if (!s.matches("[\\s\\dA-Za-z]*")) {
        throw new IllegalArgumentException("String too complicated");
    } else if(s.isEmpty()) {
        return new MorseString("","");
    }
    String result = Arrays.asList(s.split(""))
           .stream()
           .reduce("", (prev,current)-> {
                return " ".equals(current) ?
                         prev + WORD_SEPARATOR + CHAR_SEPARATOR :
                         prev + MorseCode.encode(current.charAt(0)) + CHAR_SEPARATOR;
           });

    return new MorseString(result);
}

Since Java 7, you don't need to use StringBuilder. Use the + operator and the compiler will do it for you.

In Java 8, you can use a stream to do this kind of operation. And instead of using main() to test, you should use JUnit (or another test framework).

import static org.junit.Assert.*;

public class MorseTest {

    @Test public void should_parse_morse_string () {
        MorseString morseString = MorseString.parse("testing this");

        assertEquals("- . ... - .. -. --. / - .... .. ...", morseString.toString());
        assertEquals("TESTING THIS", morseString.asString());
    }

    @Test public void should_parse_empty_string () {
        MorseString morseString = MorseString.parse("");

        assertEquals("", morseString.toString());
        assertEquals("", morseString.asString());
    }
}