HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Morse code string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codemorsestring

Problem

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 n

Solution

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 and Map

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

Code Snippets

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();
}

Context

StackExchange Code Review Q#74794, answer score: 5

Revisions (0)

No revisions yet.