Unwanted Null Replacement

Question

I am working on a English to Morse translator. When I input my sentence in English, I receive a translation filled with "null" rather than the corresponding Morse characters.

(Yes, this is homework.)

import javax.swing.JOptionPane;

public class test
{
public static void main ( String [] args )
{
    String s1 = "Morse";

    //Decide whether Morse code or English
    String decide = JOptionPane.showInputDialog("Enter 'English' for Morse to English code translation and 'Morse' for English to Morse code translation. Pay attention to Caps.");

    //Enter String
    String phrasep = JOptionPane.showInputDialog("Enter the words you wish to translate.");

    if ( decide.equals( s1 ))
        toMorse( phrasep );
    else
        toEnglish( phrasep );
}

// Translate to Morse
public static void toMorse( String phrase1 )
{
    char[] english = new char[36];

    for (  int i = 65, j = 0; i < 91; i++, j++) {
        english[j] = (char)i;
    }

    english[26] = 1;
    english[27] = 2;
    english[28] = 3;
    english[29] = 4;
    english[30] = 5;
    english[31] = 6;
    english[32] = 7;
    english[33] = 8;
    english[34] = 9;
    english[35] = 0;

    String[] morse = {".-","-...","-.-.","-..",".","..-.","--.","....","..", ".---",
            "-.-",".-..","--","-.","---",".--.","--.-",".-.","...","-","..-", 
            "...-",".--","-..-","-.--","--.."};

    //Replace spaces with |
    String phrase = phrase1.replace( "//s+", "|");

    String[] translation = new String[phrase1.length()];

    //Translate
    for ( int j = 0, t = 0, n = 1; j < phrase.length(); j++) {
        if ( phrase.substring(t, n ).equals ( english[j] ) ) {
            translation[t] = morse[j];
            t++;
            n++;
        }
    }

    String separatorp = new String( "|" );
    arrayToString ( translation, separatorp );
}

public static void toEnglish( String phrase) {
    System.out.println( phrase );
}

//Convert array to string and print translation
public static void arrayToString(String[] trans, String separator) 
{
    String result = "";
    if (trans.length > 0) {
            result = trans[0];    // start with the first element
            for (int i = 1; i < trans.length; i++)
                    result = result + separator + trans[i];
        }
    System.out.println ( result );
}
}

Nathaniel Ford · Accepted Answer

There is significant confusion about how you're writing this program. Let's take your core functionality and consider it:

public static String toMorse(String english) { ... }

Unlike your implementation, note that it returns a String. This is because you're giving it one string (the English phrase) and you want back another string (the Morse phrase). Always think first about what your data is, and write functions on that data. Don't write the function and think about what your data is second.

Consider now the essential part of the implementation of this function, your version here:

//Translate
for ( int j = 0, t = 0, n = 1; j < phrase.length(); j++) {
    if ( phrase.substring(t, n ).equals ( english[j] ) ) {
        translation[t] = morse[j];
        t++;
        n++;
    }
}

First, why are you instantiating three variables, which you fail to name (so what they mean is unclear to someone looking at it the first time) and which you increase in lockstep? The following does the same thing:

//Translate
for ( int j = 0, t=0; j < phrase.length(); j++) {
    if ( phrase.substring(t, (t+1) ).equals ( english[j] ) ) {
        translation[t] = morse[j];
        t++;
    }
}

But it's still unclear what j and t do from the name. You're really using them as an indices as you do a character-by-character conversion. If we look at the Java String API we see that you're using substring() to get a particular character... except that your start and end points are j and j+1, which means you're going to always get two characters. Two characters will never match a single character.

The documentation linked above shows another option for retrieving a character.

While we're at it, please note:

If you're following good programming style, your delimiter should be stored as a constant, not hard-coded into your function.
In one place you're using replace() to replace instances of a particular thing with another. In a second place you're exhaustively traversing an n-length list looking for a particular match. Use one method or the other. Using both is inappropriate here and confusing.
Your translation time is O(n*m), where n is the length of your phrase and m is the number of letters in your alphabet. Generally this will be considered to be O(n^2), which is bad. A Map is really what you want, because it's O(1) access time, reducing your function to O(n), where n is the length of your phrase.
Your variables need to be clearly named so you clearly know what they do. The act of naming them will force you to keep straight in your head what they mean.
You should set up your dictionaries outside the function in which you utilize them. This way you can check them for correctness independent of your translation function.

My final suggestion is that you step away from the code and try to write out, in plain English, the instructions to do this conversion. You seem hung up on details (such as pasting text to a swing front-end) that prevents you from solving the problem at hand: how to properly translate a phrase from one language to another.

Unwanted Null Replacement

Answers (2)

Related Questions