Inputting and displaying strings

Question

I have written some code for one of my assignments. However, I feel that I am repeating myself slightly in a few places. I have that niggling feeling that there is a better way to do things.

Here is the code in full:

#include <stdio.h>
#include <string.h>
#define ARR_SIZE 256

int main(void)
{
    //Declare char arrays
    char arcInputString5[ARR_SIZE];
    char arcInputString10[ARR_SIZE];
    char arcInputString15[ARR_SIZE];
    char arcInputString20[ARR_SIZE];
    int clean1, clean2, clean3, clean4, nCount;
    char buffer[ARR_SIZE];

    printf("\nPlease Input String 1 - Max Length 5: ");
    //gets(arcInputString5);
    fgets(arcInputString5, ARR_SIZE, stdin);

    for(clean1 = 0; clean1 < strlen(arcInputString5); clean1++)
    {
        if(arcInputString5[clean1] == '\n' || arcInputString5[clean1] == '\r')
        {
            arcInputString5[clean1] = '\0';
            break;
        }
    }

    printf("\nPlease Input String 2 - Max Length 10: ");
    //gets(arcInputString10);
    fgets(arcInputString10, ARR_SIZE, stdin);

    for(clean1 = 0; clean1 < strlen(arcInputString10); clean1++)
    {
        if(arcInputString10[clean1] == '\n' || arcInputString10[clean1] == '\r')
        {
            arcInputString10[clean1] = '\0';
            break;
        }
    }

    printf("\nPlease Input String 3 - Max Length 15: ");
    //gets(arcInputString15);
    fgets(arcInputString15, ARR_SIZE, stdin);

    for(clean1 = 0; clean1 < strlen(arcInputString15); clean1++)
    {
        if(arcInputString15[clean1] == '\n' || arcInputString15[clean1] == '\r')
        {
            arcInputString15[clean1] = '\0';
            break;
        }
    }

    printf("\nPlease Input String 4 - Max Length 20: ");
    //gets(arcInputString20);
    fgets(arcInputString20, ARR_SIZE, stdin);

    for(clean1 = 0; clean1 < strlen(arcInputString20); clean1++)
    {
        if(arcInputString20[clean1] == '\n' || arcInputString20[clean1] == '\r')
        {
            arcInputString20[clean1] = '\0';
            break;
        }
    }

    printf("\nThankyou For Your Inputs - They Are Shown Back To You Below\n\n");

    puts(arcInputString5);
    puts(arcInputString10);
    puts(arcInputString15);
    puts(arcInputString20);

    printf("\nThe String Lengths For Each Input Are Listed Below. \nIt is also indicated below if you have typed too many characters in");

    printf("\n%d", strlen(arcInputString5));
    printf("\n%d", strlen(arcInputString10));
    printf("\n%d", strlen(arcInputString15));
    printf("\n%d", strlen(arcInputString20));

    if(strlen(arcInputString5) > 5)
    {
        printf("\n\nString Length: %d - Exceeded Allowed Characters", strlen(arcInputString5));
    }
    else
    {
        printf("\n\nString Length: %d - NOT Exceeded Allowed Characters", strlen(arcInputString5));
    }

    if(strlen(arcInputString10) > 10)
    {
        printf("\n\nString Length: %d - Exceeded Allowed Characters", strlen(arcInputString10));
    }
    else
    {
        printf("\n\nString Length: %d - NOT Exceeded Allowed Characters", strlen(arcInputString10));
    }

    if(strlen(arcInputString15) > 15)
    {
        printf("\n\nString Length: %d - Exceeded Allowed Characters", strlen(arcInputString15));
    }
    else
    {
        printf("\n\nString Length: %d - NOT Exceeded Allowed Characters", strlen(arcInputString15));
    }

    if(strlen(arcInputString20) > 20)
    {
        printf("\n\nString Length: %d - Exceeded Allowed Characters", strlen(arcInputString20));
    }
    else
    {
        printf("\n\nString Length: %d - NOT Exceeded Allowed Characters", strlen(arcInputString20));
    }


    //printf("\n\nBelow are the strings Concatenated: \n");
    sprintf(buffer, "\n\nBelow are the strings Concatenated: \n\n>%s<>%s><%s<>%s<", arcInputString5, arcInputString10, arcInputString15, arcInputString20);
    printf("%s", buffer);
    //puts(buffer)
}

I feel that the 4 sections of this code:

for(clean1 = 0; clean1 < strlen(arcInputString20); clean1++)
{
    if(arcInputString20[clean1] == '\n' || arcInputString20[clean1] == '\r')
    {
        arcInputString20[clean1] = '\0';
        break;
    }
}

And this code:

if(strlen(arcInputString5) > 5)
{
    printf("\n\nString Length: %d - Exceeded Allowed Characters", strlen(arcInputString5));
}
else
{
    printf("\n\nString Length: %d - NOT Exceeded Allowed Characters", strlen(arcInputString5));
}

Could be shortened somehow, but I am unsure how to do this. They may not be able to, but that's why I'm posting here!

Any hints/tips are appreciated.

Answer 1

You currently have separate variables for each input. You could instead have a two-dimensional array:

char inputs[4][ARR_SIZE];

You have variables clean1, clean2, …, nCount, but only use clean1. Since they're only used as index variables in non-nested loops, you only need one.

Then you can refactor the reading parts to have a for loop that reads each input in turn. You can do something similar when printing the strings out.

For checking their lengths, you can also do something similar, but you might want to have an array of maximum lengths to check against, e.g.:

static const int max_lengths[] = { 5, 10, 15, 20 };

Answer 2

Whenever you have a self-contained chunk of code that serves an identifiable purpose, it's a good idea to package it in a function. That's true even if you aren't repeating yourself four times. The fact that all the code appears in quadruplicate makes it even more urgent to define some functions to make your code reusable.

To decide how to break up your code, the key questions to ask are:

1. What is the purpose of this chunk of code? Can you give it a name? If you have a hard time naming the operation, maybe you need to decompose it further into smaller operations.
2. What could vary? Those will be your parameters. What are the operation's outputs?

Here's how I recommend decomposing your main() function:

/**
 * Prints the prompt, then writes a '\0'-terminated string of user input to buf. 
 */
void input(const char *prompt, char *buf, int bufsize) {
    /* TODO: Exercise for you */
}

/**
 * Truncates str at the first '\n' or '\r' character.
 */
void chomp(char *str) {
    str[strcspn(str, "\n\r")] = '\0';
}

/**
 * Prints either "String Length: N - Exceeded Allowed Characters\n"
 *            OR "String Length: N - NOT Exceeded Allowed Characters\n"
 */
void report_whether_length_exceeded(const char *str, int maxlen) {
    /* TODO: Exercise for you */
}

int main() {
    char str5[ARR_SIZE], str10[ARR_SIZE], str15[ARR_SIZE], str20[ARR_SIZE];
    char buffer[ARR_SIZE];

    input("Please Input String 1 - Max Length 5:", str5, sizeof(str5));
    input("Please Input String 2 - Max Length 10:", str10, sizeof(str10));
    input("Please Input String 3 - Max Length 15:", str15, sizeof(str15));
    input("Please Input String 4 - Max Length 20:", str20, sizeof(str20));

    chomp(str5);
    chomp(str10);
    chomp(str15);
    chomp(str20);

    printf("Thank you for your inputs - they are:\n\n"
           "%s\n%s\n%s\n%s\n",
           str5, str10, str15, str20);

    printf("Their string lengths are:\n\n"
           "%d\n%d\n%d\n%d\n",
           strlen(str5), strlen(str10), strlen(str15), strlen(str20));

    report_whether_length_exceeded(str5, 5); 
    report_whether_length_exceeded(str10, 10);
    report_whether_length_exceeded(str15, 15);
    report_whether_length_exceeded(str20, 20);

    /* I assume you want to append all the strings to a buffer for fun.  Otherwise,
       you could achieve the same effect more easily just with printf().
       It would be a good habit to use snprintf() instead of sprintf() if
       it's available on your system. */
    sprintf(buffer, ">%s<>%s><%s<>%s<", str5, str10, str15, str20);
    printf("\nHere are the strings concatenated:\n\n%s\n", buffer);
}

Once you accomplish that, you could go one step further: instead of storing four strings in separate char arrays, you could hold them all in a single array of strings (that is, a two-dimensional char array). However, I consider it more important to understand how to decompose your code into functions, so I'll leave it at that.

A final note: you'll give yourself fewer headaches if you consistently end each printout with a trailing newline. Sometimes putting it at the end of the previous line, and sometimes putting it at the beginning of the next line makes your code harder to work with.