3
\$\begingroup\$

I wrote code that replaces integers from 0 to 3 with strings. I was only allowed to use getchar() and putchar(). If the input is 1, the output will become "one".

#include <stdio.h>


int main()
{


int c;            

char* arr[4] = {"zero", "one", "two","three"};

int i;

  while ((c = getchar ()) != EOF) 
    {

        if(c==0+'0'){
        char* str = arr[0];
    for (i = 0; str[i] != '\0'; i++) {

    putchar(str[i]);

}
}


else if(c==1+'0'){
char* str = arr[1];
      for (i= 0; str[i] != '\0';i++) {

    putchar(str[i]);


}
}

else if(c==2+'0'){
char* str = arr[2];
      for (i = 0; str[i] != '\0'; i++) {
        putchar(str[i]);

}
}
else if(c==3+'0'){

    char* str = arr[3];
     for (i = 0; str[i] != '\0'; i++) {
    putchar(str[i]);

}   
}



        else
        putchar(c);

   }

return 0;


}

The code is pretty long. Is there a shorter way to write it?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Mast
    Commented May 10, 2018 at 21:41

3 Answers 3

3
\$\begingroup\$

You can handle all the cases for 0 to 3 in a single piece of code. There basic idea is:

If the character is between 0 and 3, print the corresponding word, otherwise print only the character.

In C code, this reads:

if ('0' <= c && c <= '3') {
    const char *word = arr[c - '0'];
    while (*word != '\0') {
        putchar(*word);
        word++;
    }
} else {
    putchar(c);
}

With this code, it is much simpler to extend the list of words for the remaining digits. Instead of copying and adjusting a whole block of code, you only have to change the code at two places:

  • The array of the actual words, adding "four"
  • The '3' in the if clause

This can even be reduced more:

if ('0' <= c && c <= '0' + sizeof arr / sizeof arr[0]) {

After this change you only need to add the words to the array. Everything regarding sizeof is fairly advanced stuff though, so I don't expect you to fully understand this right now. But it's good to know that there is a way, at least.

By the way, this code does not swap anything. It is better to call this "replace" instead of "swap". As a programmer, you should be as precise as possible.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you. I want to get better at programming and I will take your advice to be more precise while asking questions. \$\endgroup\$
    – momonosuke
    Commented May 12, 2018 at 12:44
4
\$\begingroup\$

Review

Vertical White Space

You have way too many blank lines.
Please remove 90% of them.

Formatting

This code is horribly formatted.
Code is designed for humans to read. Part of that is making sure it is easy to read.

DRY

There is one piece of code that is repeated several times.

for (i = 0; str[i] != '\0'; i++) {
    putchar(str[i]);
}

You can simplify your code by putting this into its own function. Then calling the function when you want to print out the string.

Easier to read

Your comparisons to check for a number is overly complex:

else if(c==2+'0'){

The c is a character so simply compare the user input to the expected character:

 else if (c == '2') {
\$\endgroup\$
1
\$\begingroup\$

In addition to Roland's review, I'd like to mention good use of const.

We have:

char* arr[4] = {"zero", "one", "two","three"};

Since we never modify the strings in this array¹, we can go further and make the elements pointers to const char:

const char *arr[4] = {"zero", "one", "two", "three"};

Further, we don't want to modify the pointers themselves (assuming we don't plan to make this program multi-lingual), we can declare them as const pointers to const char:

const char *const arr[4] = {"zero", "one", "two", "three"};

As a further refinement, we can make this static, which means the array can be stored in read-only memory (like a global constant), rather than within the stack-frame of main():

static const char *const arr[4] = { "zero", "one", "two", "three" };

I know it looks a bit heavy when expressed like that, but once you understand each word in the declaration, it's very precise!


¹ In fact, we mustn't modify the strings, because a string literal such as "zero" is a constant object; the ability to assign a string literal to a plain char* variable is a historical - and undesirable - artefact of C (it dates from before the const keyword was added to the language more than 30 years ago). GCC has an option, -Wwrite-strings, to alert you to code that uses this feature, so you can add the necessary const - I recommend you use it, in addition to -Wall -Wextra, and heed its output.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.