5

I am writing a basic morse code converter in PHP which can take a string and convert it into morse code. It is using an associative array, a foreach loop, and a for loop. It works, except for some reason it outputs the morse code equivalent for '0' after each converted character. I can't figure out where the 0 is coming from. If I remove 0 from the associative array, there is no problem but I want to be able to convert numbers as well. If anyone is able to give me some feedback, that would be much appreciated.

Here is the code:

<?php
$string = "dog";
$string_lower = strtolower($string);
$assoc_array = array(
    "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"=>"--..", 
    "0"=>"-----",
    "1"=>".----", 
    "2"=>"..---", 
    "3"=>"...--", 
    "4"=>"....-", 
    "5"=>".....", 
    "6"=>"-....", 
    "7"=>"--...", 
    "8"=>"---..", 
    "9"=>"----.",
    "."=>".-.-.-",
    ","=>"--..--",
    "?"=>"..--..",
    "/"=>"-..-.",
    " "=>" ");
    for($i=0;$i<strlen($string_lower);$i++){
        foreach($assoc_array as $letter => $code){
            if($letter == $string_lower[$i]){
                echo "$code<br/>";
            }
        }
    }
?>
8se7en
  • 61
  • 1
  • 4
  • 2
    you were able to create the script above. I don't see any reason why you should not be able to pull that off. It's just some string manipulation. Though I wan't to help you, but you need to do your homework. :D – Severino Lorilla Jr. Mar 22 '16 at 09:09
  • "Locked" Code-golf canonical: [Convert a string into Morse code](https://stackoverflow.com/q/1352587/2943403) – mickmackusa Oct 20 '22 at 02:47

5 Answers5

5

The main issue is that you are doing "more" than necessary. There is no need to loop through your $assoc_array like that when you can use the string to fetch the needed data from it.

This also uses less resources, as instead of looping from a-z and 0-9 you only loop the exact amount of letters / numbers / spaces required.

/*Rest of your code above*/
for($i=0;$i<strlen($string_lower);$i++){
    echo (isset($assoc_array[$string_lower[$i]])) ? $assoc_array[$string_lower[$i]] . '<br />' : 'ERROR';       
} 

Since your array contains everything from a-z and 0-9 you can easily just call the required letters without worrying about missing data.

Edit: Added a isset() check, it will hardly be needed as the $assoc_array covers every needed letter / number, but better safe than sorry. (Credit to @Farkie for reminding me)

Epodax
  • 1,828
  • 4
  • 27
  • 32
  • You should check that $string_lower[$i] exists first – Farkie Mar 22 '16 at 09:22
  • @Farkie Yes, you could argue that, but since OP has pretty much every letter / number covered there's hardly a chance that it isn't set / there, but alas, you are right and I will amend my answer to it. (I assume you mean check that `$string_lower[$i]` exist in the `$assoc_array`) – Epodax Mar 22 '16 at 09:24
  • Thanks, this makes much more sense! I think the reason why I used a foreach loop was because I didn't think I could use indexing with an associative array (I thought I could only refer to its key) so I was using a foreach loop to go through each value one by one and over complicating it. I'm not sure where I got that idea from but I'm glad I've got cleared up now. Thanks again! – 8se7en Mar 22 '16 at 09:25
  • I will add code to check that $string_lower[$i] exists first but at this point I was just trying to get the basics working and go from there. Most of the letters/numbers are included in the array. – 8se7en Mar 22 '16 at 09:26
2

The simplest fix is simple to add a 'break' after the echo:

foreach($assoc_array as $letter => $code){
                if($letter == $string_lower[$i]){
                        echo "$code<br/>";
                        break;
                }
        }

The real problem is that the 0 is evaluating to false, which means that when it's looping over it, it's going to be a truthy (false == false).

You can solve it even better by doing an identical (===) match:

foreach($assoc_array as $letter => $code){
                if($letter === $string_lower[$i]){
                        echo "$code<br/>";
                        break;
                }
        }
Farkie
  • 3,307
  • 2
  • 22
  • 33
1

You can also use a bit of php functional programming, such as the array_reduce() function http://php.net/manual/en/function.array-reduce.php

Avoiding all those ugly for loops and simplifying your code quite a lot:

$convert = function($carry, $item) {
    $table = array(
        "a" => ".-",
        "b" => "-...",
        "c" => "-.-.",
        "d" => "-..",
        "g" => "--.",
        "o" => "---");
    // Get the correspondent value for the given letter
    $morse = $table[$item];
    // Return the string with appended morse character
    return $carry . $morse;
};

// Split 'dog' into an array, then apply a reduce to convert it to morse
array_reduce(str_split('dog'), $convert);
// ➜  ~ php morse.php                                                     
// -..-----.
João Gonçalves
  • 3,903
  • 2
  • 22
  • 36
1

I know it's quite a while since the question was posted and the answers where given. I thought I would add this function I wrote for people finding this question in the future.

Code

/**
 * Convert string to morse
 *
 * @param string $string
 * @return string
 */
function str_to_morse(string $string) {
    // Make the string lowercase and create an array of the characters
    $stringParts = str_split(strtolower($string));

    // Define the dictionary
    $morseDictionary = [
        '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' => '--..',
        '0' => '-----',
        '1' => '.----',
        '2' => '..---',
        '3' => '...--',
        '4' => '....-',
        '5' => '.....',
        '6' => '-....',
        '7' => '--...',
        '8' => '---..',
        '9' => '----.',
        '.' => '.-.-.-',
        ',' => '--..--',
        '?' => '..--..',
        '/' => '-..-.',
        ' ' => ' ',
    ];

    $morse = '';
    foreach ($stringParts as $stringPart) {
        if (array_key_exists($stringPart, $morseDictionary)) {
            $morse .= $morseDictionary[$stringPart] . '<br />';
        }
    }

    return $morse;
}
Peter
  • 8,776
  • 6
  • 62
  • 95
0

This appears to the be earliest occuring Morse code encode/decode question for the language of PHP, so I suppose this is the best place to indicate that PHP has a native function that can translate substrings using an associative array.

strtr() will seek out the longest qualifying match and replace the found substring. strtr() will not replace replacements unlike str_replace(). preg_ techniques do not appear to be warranted for this task.

Once you have your input string forced to lowercase (to match your lookup/translation array keys) and you have some sort of delimiting character between each character, you don't need a loop -- only need strtr().

Code to Morse-encode: (Demo)

$string = "d o g";
echo strtr($string, CHAR_TO_MORSE);
// output: -.. --- --.

Inverting the translation is as simple as flipping the lookup/translation array.

Code to Morse-decode (Demo)

$string = "-.. --- --.";
echo strtr($string, array_flip(CHAR_TO_MORSE));
// output: d o g

The lookup constant (based on the asker's array of translations) can be declared like this:

const CHAR_TO_MORSE = [
    "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" => "--..",  "0" => "-----",  "1" => ".----",  "2" => "..---",  "3" => "...--", 
    "4" => "....-", "5" => ".....",  "6" => "-....",  "7" => "--...",  "8" => "---..", 
    "9" => "----.", "." => ".-.-.-", "," => "--..--", "?" => "..--..", "/" => "-..-.",
];
mickmackusa
  • 43,625
  • 12
  • 83
  • 136