10
\$\begingroup\$

Challenge

Write a program which reads a file containing Morse Code and outputs the conversion.

Specifications

  1. The first argument is a path to a file.
  2. The file contains multiple lines.
  3. Each line is a string of morse code.
    • Each letter is separated by a space.
    • Each word is separated by two spaces.

Constraints

  1. The input file is correctly formatted.
  2. The Morse Code strings are alphanumeric.

Sample Input

.- ...- ..--- .-- .... .. . -.-. -..-  ....- .....  
-... .... ...--

Sample Output

AV2WHIECX 45
BH3

Source

My Solution:

import java.io.File;
import java.io.FileNotFoundException;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;

public class Solution {
    private static final Map<String, Character> morseAlphabet = new HashMap<>();

    static {
        morseAlphabet.put(".-", 'A');
        morseAlphabet.put("-...", 'B');
        morseAlphabet.put("-.-.", 'C');
        morseAlphabet.put("-..", 'D');
        morseAlphabet.put(".", 'E');
        morseAlphabet.put("..-.", 'F');
        morseAlphabet.put("--.", 'G');
        morseAlphabet.put("....", 'H');
        morseAlphabet.put("..", 'I');
        morseAlphabet.put(".---", 'J');
        morseAlphabet.put("-.-", 'K');
        morseAlphabet.put(".-..", 'L');
        morseAlphabet.put("--", 'M');
        morseAlphabet.put("-.", 'N');
        morseAlphabet.put("---", 'O');
        morseAlphabet.put(".--.", 'P');
        morseAlphabet.put("--.-", 'Q');
        morseAlphabet.put(".-.", 'R');
        morseAlphabet.put("...", 'S');
        morseAlphabet.put("-", 'T');
        morseAlphabet.put("..-", 'U');
        morseAlphabet.put("...-", 'V');
        morseAlphabet.put(".--", 'W');
        morseAlphabet.put("-..-", 'X');
        morseAlphabet.put("-.--", 'Y');
        morseAlphabet.put("--..", 'Z');
        morseAlphabet.put("-----", '0');
        morseAlphabet.put(".----", '1');
        morseAlphabet.put("..---", '2');
        morseAlphabet.put("...--", '3');
        morseAlphabet.put("....-", '4');
        morseAlphabet.put(".....", '5');
        morseAlphabet.put("-....", '6');
        morseAlphabet.put("--...", '7');
        morseAlphabet.put("---..", '8');
        morseAlphabet.put("----.", '9');
    }

    public static void main(String[] args) {
        if (args.length != 1) {
            System.err.println(args.length > 1 ? "Excessive arguments, only the first will be considered" : "No arguments provided.");
            System.exit(1);
        }

        try (Scanner fileScanner = new Scanner(new File(args[0]))) {
            while (fileScanner.hasNextLine()) {
                System.out.println(decode(fileScanner.nextLine()));
            }
        } catch (FileNotFoundException fnfe) {
            fnfe.printStackTrace();
        }
    }

    private static String decode(String morse) {
        StringBuilder decoded = new StringBuilder();

        String[] words = morse.split("\\s{2}");
        for (String word : words) {
            decoded.append(' ');
            String[] letters = word.split("\\s");
            for (String letter : letters) {
                decoded.append(morseAlphabet.get(letter));
            }
        }

        return decoded.substring(1);
    }
}

Tests:

import org.junit.Test;
import static org.junit.Assert.assertEquals;

public class TestMorseConversion {
    @Test
    public void testCodeReviewMorse() {
        assertEquals(Morse.decode("-.-. --- -.. .  .-. . ...- .. . .--"), "CODE REVIEW");
    }

    @Test
    public void test2ndMonitor() {
        assertEquals(Morse.decode("..--- -. -..  -- --- -. .. - --- .-."), "2ND MONITOR");
    }

    @Test
    public void testFromCodeEval() {
        assertEquals(Morse.decode("..-. .-. --- --  -.-. --- -.. . . ...- .- .-.."), "FROM CODEEVAL");
    }
}

Note: The Morse class simply contains the map and decode method utilized above to facilitate testing.

\$\endgroup\$
  • \$\begingroup\$ I'm missing a part where you do with output: "sorted on length in descending order". Bug? (or just too early morning for me, being blind?) \$\endgroup\$ – Ped7g Jul 18 '16 at 10:11
  • \$\begingroup\$ Not a bug. A total mis-representation of the the challenge, haha. Thanks for that, fixing it! \$\endgroup\$ – Legato Jul 18 '16 at 11:18
  • \$\begingroup\$ Just wondering, is this for Java only? I have a few languages I'm interested in but don't know yet, and was thinking about doing this in one of those. A little exercise like this is a great way to get familiar with the basics of a new language. Would I post that as an answer here or should it be a separate "question"? Thanks! \$\endgroup\$ – Michael Geary Jul 20 '16 at 19:30
  • \$\begingroup\$ I find using programming challenge sites like CodeEval (the one this is from) is a great way to learn the basics of a language. So solve it in a language you like, and post it on here as a question. If while solving it you find you have some insight that may be applicable to my current algorithm then by all means share, it is very welcome. Happy hacking! Also if no one has told you this yet, welcome to Code Review. \$\endgroup\$ – Legato Jul 20 '16 at 19:32
  • \$\begingroup\$ Thank you, @Legato, and especially for the kind wishes. Happy hacking to you too! \$\endgroup\$ – Michael Geary Jul 20 '16 at 19:34
6
\$\begingroup\$

Debatable ... and a bug?

constructs like these are one of those things that personal-preference plays in to, but consider:

    if (args.length != 1) {
        System.err.println(args.length > 1 ? "Excessive arguments, only the first will be considered" : "No arguments provided.");
        System.exit(1);
    }

Error handling is boiler-plate that can be made more readable by separating the error conditions out explicitly. In checking/refactoring this, I found an apparent bug, too (which was hard to spot because of the compound exit condition). The bug is that if the arg-count is more than 1, you print the message "Excessive arguments, only the first will be considered" but then terminate anyway..... so much for ignoring the subsequent args... I would prefer:

private static final void terminate(String message) {
    System.err.println(message);
    System.exit(1);
}

.....

    if (args.length == 0) {
        terminate("No arguments provided");
    }
    if (args.length > 1) {
        terminate("Too many arguments provided. Expect exactly 1");
    }

A distinct advantage I find when you separate out the error conditions, is that maintaining them is simpler, especially when using version-control because the diffs are simpler to understand.

Challenge

Programming challenges are often time-constrained. While for most purposes your code is fast enough, there's a simple technique that can significantly improve performance - do bulk operations (at the expense of using memory). Specifically, it's typically better to "accumulate" the output in a "StringBuilder" and then have a single System.out.println() at the end. The in-the-loop println you currently have will cause the system to "flush" each line, and often that amounts to significant portions of your program time (more than half?).

Similarly, reading in the entire input file in one "gulp" is often faster than a Scanner, or some other operation.

Obviously, "huge" inputs may be a problem, in theory, but I have never yet been found victim to conditions like that in challenges.

Gulping

I would consider using a gulp function to read the entire file - the Java 8 java.nio.files package can help here, and I would choose:

List<String> data = Files.readAllLines(Paths.get(args[0]));

Splitting

A trick of loading an empty-string in to your decode-routine to output a space, will allow you to use a regex to split, and process each line, in a stream:

private static final Pattern charsplit = Pattern.compile(" ");

and then:

String out = charsplit.splitAsStream(line)
             .map(letter -> morseAlphabet.get(letter))
             .collect(Collectors.joining(""));

You can then append that out with a newline, to the output buffer... so that the whole thing ends up looking a little like (untested - see conclusion for working code):

String output = Files.readAllLines(Paths.get(args[0])).stream()
     .map(line -> charsplit.splitAsStream(line)
                           .map(letter -> morseAlphabet.get(letter)
                           .collect(Collectors.joining("")))
     .collect(Collectors.joining("\n");

System.out.println(output);

I would probably take that inner stream and make a real Function<String,String> out of it. I would also make the printing a separate function ... it should not be part of the parsing/decoding.

Conclusion

I put together a working example program here: https://ideone.com/hCpUA1

Note that the version of the code I was happiest with separated out the various functions, and converted the Map<String, Character> to a Map<String, String>. Note that the map also has the "Space" value in it.

Additionally, the reading of the file, and converting to List<String> is expected to be outside the function (in the main method) which cannot be done in ideone.com.

Here's the code in full:

import java.util.*;
import java.lang.*;
import java.io.*;
import java.util.stream.*;
import java.util.regex.Pattern;


class Morse
{

    private static final Map<String, String> morseAlphabet = new HashMap<>();

    static {
        morseAlphabet.put("", " ");
        morseAlphabet.put(".-", "A");
        morseAlphabet.put("-...", "B");
        morseAlphabet.put("-.-.", "C");
        morseAlphabet.put("-..", "D");
        morseAlphabet.put(".", "E");
        morseAlphabet.put("..-.", "F");
        morseAlphabet.put("--.", "G");
        morseAlphabet.put("....", "H");
        morseAlphabet.put("..", "I");
        morseAlphabet.put(".---", "J");
        morseAlphabet.put("-.-", "K");
        morseAlphabet.put(".-..", "L");
        morseAlphabet.put("--", "M");
        morseAlphabet.put("-.", "N");
        morseAlphabet.put("---", "O");
        morseAlphabet.put(".--.", "P");
        morseAlphabet.put("--.-", "Q");
        morseAlphabet.put(".-.", "R");
        morseAlphabet.put("...", "S");
        morseAlphabet.put("-", "T");
        morseAlphabet.put("..-", "U");
        morseAlphabet.put("...-", "V");
        morseAlphabet.put(".--", "W");
        morseAlphabet.put("-..-", "X");
        morseAlphabet.put("-.--", "Y");
        morseAlphabet.put("--..", "Z");
        morseAlphabet.put("-----", "0");
        morseAlphabet.put(".----", "1");
        morseAlphabet.put("..---", "2");
        morseAlphabet.put("...--", "3");
        morseAlphabet.put("....-", "4");
        morseAlphabet.put(".....", "5");
        morseAlphabet.put("-....", "6");
        morseAlphabet.put("--...", "7");
        morseAlphabet.put("---..", "8");
        morseAlphabet.put("----.", "9");
    }


    private static final Pattern charsplit = Pattern.compile(" ");

    private static final String decodeLine(String line) {
        return charsplit.splitAsStream(line)
                   .map(letter -> morseAlphabet.get(letter))
                   .collect(Collectors.joining(""));
    }

    public static final String decode(List<String> data) {
        String output = data.stream()
             .map(Morse::decodeLine)
             .collect(Collectors.joining("\n"));
        return output;
    }

    public static void main (String[] args) throws java.lang.Exception {
        String out = Morse.decode(Arrays.asList(
               "-.-. --- -.. .  .-. . ...- .. . .--",
               "..--- -. -..  -- --- -. .. - --- .-.",
               "..-. .-. --- --  -.-. --- -.. . . ...- .- .-.."));
        System.out.println(out);
    }

}
\$\endgroup\$
  • \$\begingroup\$ Edited to correct some bugs in the sugested code, and added a working example in ideone. \$\endgroup\$ – rolfl Jul 17 '16 at 21:32
  • \$\begingroup\$ If huge input files are expected, I would refrain from loading whole file first into memory (it will take a time, and you risk OOM crash). As the file I/O is the slow part in this case, you can easily process the input during reading the file (in parallel), and prepare the output. So "somewhat" buffered I/O would be IMO best, like 4-16k buffers. Things is, that you can't output it right away, as the output has to be sorted, so you already have to store whole output in memory first anyway. \$\endgroup\$ – Ped7g Jul 18 '16 at 10:10
  • \$\begingroup\$ Good considerations, @Ped7g. It need not be sorted though, apologies again for that misrepresentation. I follow a format for describing challenges and somehow forgot to update the most important field. \$\endgroup\$ – Legato Jul 18 '16 at 11:24
1
\$\begingroup\$

The morseAlphabet map would be better as an immutable constant:

private static final Map<String, String> MORSE_ALPHABET;
static {
    Map<String, String> m = new HashMap<>();
    m.put(".-",     "A");
    m.put("-...",   "B");
    m.put("-.-.",   "C");
    m.put("-..",    "D");
    m.put(".",      "E");
    …
    m.put("----.",  "9");
    MORSE_ALPHABET = Collections.unmodifiableMap(m);
}

I'm not a fan of splitting the input into words, and the words into characters. Since you are using regular expressions, it would be a good idea to follow the idiom (which unfortunately uses StringBuffer rather than StringBuilder).

There is a way to obtain a generous estimate of the buffer size, so it would be a good idea to do so.

private static final Pattern PATTERN = Pattern.compile("([.-]+)| ( ?)");

public static String decode(String morse) {
    StringBuffer decoded = new StringBuffer(morse.length() / 2);
    Matcher m = PATTERN.matcher(morse);
    while (m.find()) {
        String out = MORSE_ALPHABET.get(m.group(1));
        m.appendReplacement(decoded, out != null ? out : m.group(2));
    }
    return decoded.toString();
}
\$\endgroup\$

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.