-1

Trying to write a program but it is giving me a weird error and I cant seem to figure out what it wrong. For the assignment we use a program called code step by step. I'm not sure if it is a program issue or if the code is wrong. here is my code:

char* acronym(char* word){
char result[50];
strcpy(result, "");
int index = 0;
for(int i = 0; i < strlen(word); i++){
    if(isalpha(word[i]) && !isalpha(word[i-1])){
        char upper = toupper(word[i]);
        char * add = &upper;
        strncat(result, add, 1);
    }
}
char *p = &result;
printf("%s\n", p);
return p;

} here is an image of what it output.

It returns values that don't make sense like "P????","`?@u?", "0>???". and the wanted output the acroyms is listed as an unexpected output. Sorry if this is a repost but I couldn't find any relevant questions. I do get a few errors about pointers but the code still works on a online compiler.

Edit: Theses are the warning that I get from the compiler however it still runs (on the compiler) main.c:17:12: warning: implicit declaration of function ‘isalpha’ [-Wimplicit-function-declaration] main.c:18:26: warning: implicit declaration of function ‘toupper’ [-Wimplicit-function-declaration] main.c:23:15: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types] main.c:28:1: warning: return type defaults to ‘int’ [-Wimplicit-int]

Edit: For this homework assignment we are only able to write a single function. I don't know what the rest of the program on the back end looks like. The program is called codestepbystep this is what I see. https://i.stack.imgur.com/6Lddh.jpg

BENJAMIN WEED
  • 5
  • 3
  • To start with: Your function returns the address of the array `result`. But `result` is a local variable inside the function. So `result` will not exist once the function returns. **Never** return the address of a local variable. – Support Ukraine Feb 07 '21 at 07:21
  • 1
    Then this part: `isalpha(word[i-1]` What will happen when `i` is zero? – Support Ukraine Feb 07 '21 at 07:22
  • Also, the `is*`/`to*` functions are not easy to use correctly. You ought to write `isalpha((unsigned char)word[i])` and `toupper((unsigned char)word[i]);` - or these should be unsigned char to begin with. `strncat` is an inefficient way to write this and prone to mistakes anyway - for example at no point are you testing if the `result` is sufficiently big - once you make that check then you notice that you'd just insert characters at indexes and add the terminating zero and there is no point for `strncat`. – Antti Haapala -- Слава Україні Feb 07 '21 at 07:48
  • Also you need to add **all of the error messages** to the question as they're relevant. – Antti Haapala -- Слава Україні Feb 07 '21 at 07:48
  • `char *p = &result; printf("%s\n", p);` this is nonsense. just do `printf("%s\n", result);` – Antti Haapala -- Слава Україні Feb 07 '21 at 07:49
  • https://stackoverflow.com/questions/11656532/returning-an-array-using-c – Antti Haapala -- Слава Україні Feb 07 '21 at 07:49
  • Use a good editor, like [GNU emacs](https://www.gnu.org/software/emacs/), to type your source code. Then enable warnings and debug info in your compiler. With [GCC](http://gcc.gnu.org/) compile with `gcc -Wall -Wextra -g`. Once you got no warnings, use [GDB](https://www.gnu.org/software/gdb/) or some other debugger to understand the behavior of your program. And use [valgrind](http://valgrind.org/) – Basile Starynkevitch Feb 07 '21 at 10:25
  • BTW, in 2021 [UTF-8 is everywhere](http://utf8everywhere.org/). What should your program do with an input like : Être ou ne pas être? – Basile Starynkevitch Feb 07 '21 at 10:29
  • @Antti Haapala that particular line was my attempt to debug it I forgot to remove it when I posted here. – BENJAMIN WEED Feb 08 '21 at 01:14
  • @4386427 So how would the correct way to do this within as single function? Is there a subject area I should be googling? We are only able to write a single function. https://imgur.com/ORjk1xX is what we see. The answer is probably already out there but I'm just learning to code in c so I don't really know the proper search terms. – BENJAMIN WEED Feb 08 '21 at 01:21

1 Answers1

1

People have already addressed your fundamental issues in the comments. If you're still stuck, here's a working program to help you get around your issues. I made two fundamental changes. First, as others have discussed, you can't return an array that is a local variable. So, I declared it in main and passed it as an argument. Second, the check to see if the previous character is not alphabetic has an edge case where you're on the first character aka index 0. In that case, you can't check one character back since that would bring you to the nonexistent index of -1. So, I have a special while loop at first to deal with the first letter of the acronym.

After that first letter, something similar to what you had works fine.

I'm not saying my function is the best way to do it or the only way, and certain things I don't address. For example, I don't check if the result array ever gets overflow. However, for your purposes, I just made result be large enough in main so that it wouldn't happen for the example phrases your provided. But, based on what you provided it works and I hope it helps clears things up.

#define _CRT_SECURE_NO_WARNINGS

#include <stdio.h>
#include <string.h>
#include <ctype.h>

char* acronym(char *word);

int main(int argc, char **argv) {
    char phrases[8][100] = { " automatic teller machine", "personal identification number",
        "computer science", "merry-go-round", "All my Children", "Trouble Assets Relief Program",
        "--quite-- confusing - punctuation-", "loner " };
    unsigned phrasesLength = sizeof(phrases) / sizeof(phrases[0]);
    for (unsigned i = 0; i < phrasesLength; i++) {
        printf("Phrase: %s\n", phrases[i]);
        printf("Acronym: %s\n\n", acronym(phrases[i]));
    }
    return 0;
}

char* acronym(char* word) {
    char wordCopy[500]; // wil hold a copy of word. Make large enough to avoid overflow
    unsigned wordLength = strlen(word);

    /* To use a function to copy word into wordCopy, you could do
       wordCopy[i] = '\0';
       strcpy(wordCopy, word);
    */
    // Here's how to do the copy manually
    for (unsigned i = 0; i < wordLength + 1; i++)
        wordCopy[i] = word[i];
    
    // wordCopy is now a copy of word

    unsigned indexWordCopy = 0; // start at first character of phrase
    unsigned indexResult = 0;   // start at first character of word
                                // this will override the characters that were
                                // there previously

    // iterate through word copy character by character. Store the acronym in word
    // which right now contains the original phrase
    while (!isalpha(wordCopy[indexWordCopy]))
        indexWordCopy++;
    word[indexResult++] = toupper(wordCopy[indexWordCopy++]);
    word[indexResult] = '\0';

    while (indexWordCopy < wordLength) {
        if (isalpha(wordCopy[indexWordCopy]) && !isalpha(wordCopy[indexWordCopy - 1])) {
            word[indexResult++] = toupper(wordCopy[indexWordCopy]);
            word[indexResult] = '\0';
        }
        indexWordCopy++;
    }
    return word;
}

OUTPUT

Phrase:  automatic teller machine
Acronym: ATM

Phrase: personal identification number
Acronym: PIN

Phrase: computer science
Acronym: CS

Phrase: merry-go-round
Acronym: MGR

Phrase: All my Children
Acronym: AMC

Phrase: Troubled Assets Relief Program
Acronym: TARP

Phrase: --quite-- confusing - punctuation-
Acronym: QCP

Phrase:  loner
Acronym: L
Ben
  • 195
  • 7
  • Thanks for this, I tried some of the above suggestions but they only worked outside of the required compiler. We use a program called code step by step for our grade. when I tryed out your code it gave and error. too few arguments to function 'acronym'. and "return makes integer from pointer without a cast return result;" https://imgur.com/a/ALXQ7Q7 here is what I see, I have been working to modify my code or yours to make it work but I'm a loss as I'm not sure what is happening on the back end from the site itself. – BENJAMIN WEED Feb 08 '21 at 01:08
  • The first error ```return makes integer from pointer without a cast``` is because you didn't copy my function correctly. Notice how the return value I wrote it "char*" but you made it be char. In regards to the second error, I didn't realize your function could only have one parameter. Whatever test is used behind the scenes only passes a single argument to the function, but now since you've made it have 2 parameters, this causes an error. Your function is written to accept two arguments, but only one argument is passed to it. – Ben Feb 08 '21 at 03:26
  • To make the function work, you can switch it back to a single parameter ```word```. In the beginning, copy ```word``` into some local character array in the function, use that local array to go character by character through the phrase, and store the acronym in ```word``` and return ```word```. Since word is the only array parameter allowed, you must return the acronym in word. – Ben Feb 08 '21 at 03:29
  • Thanks. so I changed it up a bit wondering if you could give me some more input, you define result in the first part (that I cant have) if I move the definition down into the function is it now local and that was one of the issues last time? That I was trying to return a local variable? Also when I just try to print it becomes a console output which shows the right thing should be returned but it is retuning "?". https://imgur.com/a/Iuwmoyo is what I see. I really appreciate the help – BENJAMIN WEED Feb 08 '21 at 03:37
  • Okay, you're not doing what I said to do. That's my fault, I probably wrote too much. Here's what you need to do. 1) If a function is going to return an array that is not dynamically allocated, you MUST return an array that is passed as a parameter. In this case, that is ```word```. 2) At the beginning of the function, copy the characters from ```word``` into some local character array. Then it's that local character array that you loop through checking the characters, and ```word``` is the array which will store the acronym like SCUBA. – Ben Feb 08 '21 at 03:39
  • let me edit my original post showing you what I mean – Ben Feb 08 '21 at 03:41
  • would I use memcpy or strncpy be a better way to copy word into a local array? – BENJAMIN WEED Feb 08 '21 at 03:43
  • Sorry I wrote my reply didnt see your post about word. but am working on that now. Thank you that would be very helpful. – BENJAMIN WEED Feb 08 '21 at 03:44
  • I just edited my post. Yes, you could a function to do the copying. But, since you're new to programming, only do that if you would know how to do it yourself manually without a function. If not, try to figure out how to do it manually. Else, you don't really understand how it actually works. – Ben Feb 08 '21 at 04:11
  • Thank you so much, this has been really helpful. The code is clear and I feel like I get it but when it put it into codestepbystep it says the return is NULL. Is it possible that the back end or the website is wrong? The code works fine on the other IDE. By the end of the function ` word` is the acroym but is is a array should I change it to a string and return that? Sorry for all these questions. So a A "string" in C is an array of characters followed by a sentinel character (NULL terminator)., is that what is beign returned for some reason the ` NULL` – BENJAMIN WEED Feb 08 '21 at 04:54
  • Please upload a picture of the code (so I can check you didn't make any copy mistakes) and a picture of the errors. – Ben Feb 08 '21 at 06:23
  • https://imgur.com/a/WEjMXhF I tried it both way with the function and with the manual by it shows the same error. – BENJAMIN WEED Feb 08 '21 at 06:59
  • Ok, I think I know the issue. Instead of storing the phrase in a character array, and passing the character array as the argument to acronym, they are passing a c-string literal which means "word" isn't actual a variable that can be modified and returned. So, instead of doing ```acronym(word);``` where word is some character array you've made previously, they do ```acronym("this is the phrase");``` To fix this, use dynamic memory allocation in your function to create a new array and store the acronym in it. Then return that dynamically allocated array and NOT word. – Ben Feb 08 '21 at 08:17