HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Reverse input program in C99

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
inputprogramreversec99

Problem

I am writing simple program to reverse its input and output the reversed version. I came up with this code. So how can I improve it? Something tells me that int lengthSoFar is bad in both printArray and reverse functions.

#include 
#include 

#define ARRSIZE 1000

//function declarations...
char *reverse(char *a, int l);

void printArray(char *a, int l);

int main(void) {
    int c = 0;
    int i = 0;
    char buff[ARRSIZE];
    while ((c = getchar()) != EOF) {
        if (i > ARRSIZE) printArray(reverse(buff, i), i);
        if (c != '\n') {
            buff[i]=c;
            i++;
        }else{
            printArray(reverse(buff, i), i);
            memset(buff, 0, sizeof buff);
            i=0;
        }
    }
    return 0;
}

char *reverse(char *array, int lengthSoFar) {
    int i = 0;
    int j = lengthSoFar - 1;
    static char arrayToReturn[ARRSIZE];
    while (i = 0) {
        arrayToReturn[i] = array[j];
        i++;
        j--;
    }
    return arrayToReturn;
}

void printArray(char *array, int lengthSoFar) {
    for (int i = 0; i < lengthSoFar; i++) {
        putchar(array[i]);
    }
}

Solution

Take a destination for the reversed string

Instead of a local pointer, make reverse a void function:

void reverse(char * dst, const char * src, size_t length) {
    for(size_t i = 0; i < length; ++i) {
        dst[i] = src[length - i - 1];
    }
}


The names dst (destination) and src (source) and their order stem from standard library functions like strncpy.

size_t is an unsigned integer type, and large enough to index an array that spans your whole memory. We're allowed to define size_t i in the for loop, therefore our variable doesn't live any longer than necessary.

But you already did that for printArray, so I guess you knew that :)

void printArray(const char * array, size_t length) {
    for (size_t i = 0; i < length; i++) {
        putchar(array[i]);
    }
}


It's good practice to use const * if you don't want to change the data by accident.

Use a function to ask for the string

To keep your main sleek and your code more reusable, provide a function to get the string from the user instead:

int getInput(char * dst, size_t max_len) {
    int length = 0;
    int c;

    while((c = getchar()) != EOF && length < max_len) {
        if(c == '\n')
            return length;

        dst[length++] = c;
    }
    return c == EOF ? EOF : length;
}


This will return EOF if getchar() returned EOF, and the number of read characters otherwise.

Putting all back together

I omit the functions I've already covered above, but if we use them, we end up with the following program:

#include 
#include 

#define ARRSIZE 1000

int getInput(char * dst, size_t max_len);
void reverse(char * dst, const char * src, size_t length);
void printArray(const char * array, size_t length);

int main(void) {
    int length = 0;
    char buff[ARRSIZE];
    char reversed[ARRSIZE];

    while ((length = getInput(buff, ARRSIZE)) != EOF) {
        reverse(buff, reversed, length);
        printArray(reversed, length);
    }
    return 0;
}


Which, in my opinion, is rather readable. Note that your memset wasn't necessary. After all, we're going to overwrite the values in buff with the next input.

Code Snippets

void reverse(char * dst, const char * src, size_t length) {
    for(size_t i = 0; i < length; ++i) {
        dst[i] = src[length - i - 1];
    }
}
void printArray(const char * array, size_t length) {
    for (size_t i = 0; i < length; i++) {
        putchar(array[i]);
    }
}
int getInput(char * dst, size_t max_len) {
    int length = 0;
    int c;

    while((c = getchar()) != EOF && length < max_len) {
        if(c == '\n')
            return length;

        dst[length++] = c;
    }
    return c == EOF ? EOF : length;
}
#include <stdio.h>
#include <string.h>

#define ARRSIZE 1000

int getInput(char * dst, size_t max_len);
void reverse(char * dst, const char * src, size_t length);
void printArray(const char * array, size_t length);

int main(void) {
    int length = 0;
    char buff[ARRSIZE];
    char reversed[ARRSIZE];

    while ((length = getInput(buff, ARRSIZE)) != EOF) {
        reverse(buff, reversed, length);
        printArray(reversed, length);
    }
    return 0;
}

Context

StackExchange Code Review Q#143241, answer score: 5

Revisions (0)

No revisions yet.