# Thread: My function is logical?

1. ## My function is logical?

Im doing a C program to decode morsetext from a file and Im struggling a bit. Well, basically Ive stored into a struct the letters/numbers and their respective morse symbols that were into a file.

Code:
```typedef struct{
char letters[5]; /* store the letters/numbers. Get it as a string but only use the first */
char lettersMorse[8];/* store the respective symbols morse of the letters/numbers */
}alphabet;
alphabet l[37];```
and the thing is that after decode the text there will be no spaces between characters. But in the encoded text from the file each letters is separated by 3 spaces and the words by 5.

lijke this:
Code:
`..---         --   .   ...         ..---         ...   ---   -.`
below theres the function I did. Its not working at all so I would like to know if anyone can help me to see if the logical is right or what is my mistake. Id be very grateful!

Code:
```void decode(const char *line){

int i,p, j;
char sentence[200] = {'\0'};

while(i<strlen(line)-1){ //read all the lines from the file
while (line[i]!= '\0'){
if(line[i] == '.' || line[i] == '-'){
//copy the line in morse from the file to the variable created
while(line[i] == '.' || line[i] == '-'){
sentence[p] = line[i];
i += 1;
++p;
}//end of loop

for (j=0; j<37; j++){
if (strcmp(sentence,l[j].lettersMorse)==0){
printf("%c", l[j].letters[0]);}
}
}
//check the spaces
else if(line[i]== ' '){
if(line[i+1]== ' '){
if(line[i+2]== ' '){
if((line[i+3]== ' ')&&(line[i+4]==' ')){
printf("%c", " ");
}else{
i++;}
}
}
}
}
}
}```

2. I think it is better if you use a binary tree:

3. Code:
```char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};

void decode(char *string) {
int current = 0, spaces = 0;
int count = 0, sencount = 0;
char characters[8], sentence[200];
while (current < strlen(string)+1) {
if (string[current] == '.' || string[current] == '-') {
if (spaces > 3) sentence[sencount++] = ' ';
characters[count++] = string[current];
spaces = 0;
} else {
if (spaces == 0) {
characters[count++] = 0;
sentence[sencount++] = morseToChar(characters);
count = 0;
};
spaces++;
};
current++;
};
sentence[sencount++] = '\0';
printf("%s",sentence);
};```

Originally Posted by Structure
Code:
```    sentence[sencount++] = '\0';
printf("%s",sentence);
};```
it will only print the last line of the file, right? and not all of them?

cause in mine main function I did something like
Code:
``` FILE *fp;
char line[MAX_STRING_SIZE];
fp = fopen("encoded.txt", "r");
if (arch==NULL){
printf("ERROR");main();}
else{
while (fgets(line, MAX_STRING_SIZE, fp)!= NULL);
decode(line);
}
fclose(fp);```

5. Code:
```  char line[MAX_STRING_SIZE];
FILE* fp = fopen("encoded.txt", "r");
while (fgets(line, MAX_STRING_SIZE, fp)) decode(line);
fclose(fp);```

6. Originally Posted by Structure
Code:
```char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};

void decode(char *string) {
int current = 0, spaces = 0;
int count = 0, sencount = 0;
char characters[8], sentence[200];
while (current < strlen(string)+1) {
if (string[current] == '.' || string[current] == '-') {
if (spaces > 3) sentence[sencount++] = ' ';
characters[count++] = string[current];
spaces = 0;
} else {
if (spaces == 0) {
characters[count++] = 0;
sentence[sencount++] = morseToChar(characters);
count = 0;
};
spaces++;
};
current++;
};
sentence[sencount++] = '\0';
printf("%s",sentence);
};```
Your code in a complete program:

Code:
```#include <stdio.h>
#include <string.h>

typedef struct{
char letters[5]; /* store the letters/numbers. Get it as a string but only use the first */
char lettersMorse[8];/* store the respective symbols morse of the letters/numbers */
}alphabet;
alphabet l[37] = {
{ "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", "----." },
};

char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};

void decode(char *string) {
int current = 0, spaces = 0;
int count = 0, sencount = 0;
char characters[8], sentence[200];
while (current < strlen(string)+1) {
if (string[current] == '.' || string[current] == '-') {
if (spaces > 3) sentence[sencount++] = ' ';
characters[count++] = string[current];
spaces = 0;
} else {
if (spaces == 0) {
characters[count++] = 0;
sentence[sencount++] = morseToChar(characters);
count = 0;
};
spaces++;
};
current++;
};
sentence[sencount++] = '\0';
printf("%s",sentence);
};

int main()
{
decode("- .... .. ...    .. ...    .-    - . ... -    --- ..-.     ... - .-. ..- -.-. - ..- .-. . ...    -.-. --- -.. .    "
".- -... -.-. -.. . ..-. --. .... .. .--- -.- .-.. -- -. --- .--. --.- .-. ... - ..- ...- .-- -..- -.-- --.. ----- .---- ..--- ...-- ....- ..... -.... --... ---.. ----. "
);
puts("");
return 0;
}```
Code:
```\$ gcc -pedantic -Wall -O2 178688.c -o 178688
178688.c:53:2: warning: ISO C does not allow extra ‘;’ outside of a function [-Wpedantic]
};
^
178688.c:76:2: warning: ISO C does not allow extra ‘;’ outside of a function [-Wpedantic]
};
^
178688.c: In function ‘morseToChar’:
178688.c:53:1: warning: control reaches end of non-void function [-Wreturn-type]
};
^```
Oops.

Code:
```\$ ./178688
THIS IS S TEST OF STRUCTURES CODE EBCDEFGHIJKLMNOPQRSTUVWXYZ0123456789```
Oops (see the two places where "A" should be).

7. gcc -pedantic -Wall -O2 178688.c -o 178688

Oops
The actual code i wrote works fine.
I think you are doing it wrong.

for one thing...
in the encoded text from the file each letters is separated by 3 spaces and the words by 5.
fyi: I see where you used the function wrong. Do you ?

8. Originally Posted by Structure
The actual code i wrote works fine.
I think you are doing it wrong.
christop's compiler is right though: your use of semi-colons to "terminate" function definitions is non-standard and hence not guaranteed to compile, and morseToChar has a bug where there's a path of control that doesn't return a value (it probably should return something like a null character to indicate an input error).

9. Code:
```char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};```
Just because you use a function wrong doesn't make it flawed.

10. Originally Posted by Structure
Code:
```char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};```
for me your code works fine without the semi-colons and it should be
Code:
`for (int i=0; i<37; i++)`
'cause another way it doesnt read the letter 'A'.

11. 'cause another way it doesnt read the letter 'A'.
sure, that is because i started with one because why not? lol

morse.h
Code:
```#define MORSES 37
#define MORSEM 8

struct alphabet {
char letters[MORSEM];
char lettersMorse[MORSEM];
};
struct alphabet l[MORSES];

char morse[MORSES][MORSEM];
strcpy(morse[1], ".-");
strcpy(morse[2], "-...");
strcpy(morse[3], "-.-.");
strcpy(morse[4], "-..");
strcpy(morse[5], ".");
strcpy(morse[6], "..-.");
strcpy(morse[7], "--.");
strcpy(morse[8], "....");
strcpy(morse[9], "..");
strcpy(morse[10], ".---");
strcpy(morse[11], "-.-");
strcpy(morse[12], ".-..");
strcpy(morse[13], "--");
strcpy(morse[14], "-.");
strcpy(morse[15], "---");
strcpy(morse[16], ".--.");
strcpy(morse[17], "--.-");
strcpy(morse[18], ".-.");
strcpy(morse[19], "...");
strcpy(morse[20], "-");
strcpy(morse[21], "..-");
strcpy(morse[22], "...-");
strcpy(morse[23], ".--");
strcpy(morse[24], "-..-");
strcpy(morse[25], "-.--");
strcpy(morse[26], "--..");
strcpy(morse[27], "-----");
strcpy(morse[28], ".----");
strcpy(morse[29], "..---");
strcpy(morse[30], "...--");
strcpy(morse[31], "....-");
strcpy(morse[32], ".....");
strcpy(morse[33], "-....");
strcpy(morse[34], "--...");
strcpy(morse[35], "---..");
strcpy(morse[36], "----.");

for (int i=1; i<37; i++) {
strcpy( l[i].lettersMorse, morse[i] );
if (i < 27) l[i].letters[0] = (64+i);
if (i > 26) l[i].letters[0] = (47+(i-26));
};

};```
(it probably should return something like a null character to indicate an input error)
I see no need in adding error checks and all possibilities considering it's not my project and that was not in the requirements. It's a quick and dirty solution as a jump off point for the developer to expand if necessary.

12. Sometimes i save index 0 for data defining what is in the array or anything else just to have a space.

13. Originally Posted by Structure
Code:
```char morseToChar(char *string) {
for (int i=1; i<37; i++) {
if (strcmp(l[i].lettersMorse, string) == 0) {
return l[i].letters[0];
};
};
};```
Just because you use a function wrong doesn't make it flawed.
That's true: it's the same reason why we have undefined behaviour in C from calling functions with invalid arguments. But this means you have to document the function pre-conditions, which you didn't, so any string could be reasonably passed as an argument (or even a null pointer! But it's easy to discern that that's a pre-condition violation at a glance)

Even if you have documented pre-conditions, it's good defensive programming technique to return a testable error value (my suggestion being a null character), or alternatively to place an assertion at the end of the function to indicate that that part of the code is expected to be unreachable (so if someone does screw up the pre-condition, they'll get an assertion failure instead of say, a mysterious value; in both cases debugging becomes easier).

14. Originally Posted by Structure
I see no need in adding error checks and all possibilities considering it's not my project and that was not in the requirements. It's a quick and dirty solution as a jump off point for the developer to expand if necessary.
Returning a value from a function in all cases isn't really adding an error check. The compiler makes it pretty easy to catch where a function fails to return a value too. You don't even need the -pedantic flag for that (the -pedantic flag just pointed out the non-standard and potentially problematic practice of using a semicolon after a function's closing brace).

Originally Posted by Structure
Just because you use a function wrong doesn't make it flawed.
But you could say the decode() function is using morseToChar() wrong, since the code I added (essentially just a test harness) doesn't directly call morseToChar().

And I wouldn't say it's using a function wrong when the user of the program can very easily and unintentionally trigger this undefined behavior in your program by including a real but unhandled Morse code character in the input, like "-..-." (which comes up often in amateur radio repeater IDs). It's easy enough to return something to indicate an unknown Morse code character on input. Just put return '?'; or similar at the end of morseToChar() (errors on input will simply be converted to "?" on output).

Besides, how can a function like decode() be expected to use morseToChar() the "right" way when it's working with a string that contains arbitrary user input? Should decode() ensure that every Morse symbol is valid before asking morseToChar() to convert it? I suppose decode() could scan through the Morse code symbol list and convert a symbol only if it's valid, but that's basically doing the same thing morseToChar() does! So yes, I would call the function flawed since it requires the caller to do practically the same work as the function. (Let me put it this way: Imagine if the C standard library function strstr() required the caller to check that the "haystack" string contained the "needle" string before it can call strstr(), otherwise Bad Things will happen. THAT would be a flawed function!)

Originally Posted by Structure
sure, that is because i started with one because why not? lol
Because it's uncommon and not at all idiomatic in C to start an array at anything but 0?

15. Originally Posted by christop
And I wouldn't say it's using a function wrong when the user of the program can very easily and unintentionally trigger this undefined behavior in your program by including a real but unhandled Morse code character in the input, like "-..-." (which comes up often in amateur radio repeater IDs). It's easy enough to return something to indicate an unknown Morse code character on input. Just put return '?'; or similar at the end of morseToChar() (errors on input will simply be converted to "?" on output).

Besides, how can a function like decode() be expected to use morseToChar() the "right" way when it's working with a string that contains arbitrary user input? Should decode() ensure that every Morse symbol is valid before asking morseToChar() to convert it? I suppose decode() could scan through the Morse code symbol list and convert a symbol only if it's valid, but that's basically doing the same thing morseToChar() does! So yes, I would call the function flawed since it requires the caller to do practically the same work as the function. (Let me put it this way: Imagine if the C standard library function strstr() required the caller to check that the "haystack" string contained the "needle" string before it can call strstr(), otherwise Bad Things will happen. THAT would be a flawed function!)
Yeah, that's true. I didn't read the code in post #3 carefully enough to see that the pre-condition is basically impossible to meet without doing the work of the function. So I agree: the function is flawed as-is. It does need to return an error that must be checked by the caller; an assertion will not do.