1
\$\begingroup\$

The given code reverses an array of input characters.

The additional while loop in reverse() function is unnecessary I guess. Any other method or way to remove it?

I would prefer to keep reverse() as void.

#include <stdio.h>

#define MAXLEN 65535 /* maximum length of input line */

void reverse(char s[], int l);

/* reverses given input line */

void main()
{
    int i, c;
    char line[MAXLEN];

    i = 0;

    while(i < MAXLEN-1 && (c = getchar()) != EOF && c != '\n')  /* input characters in array */
    {
        line[i] = c;
        ++i;
    }

    reverse(line, i);
    printf("%s", line);
}

void reverse(char s[], int l)
{
    char rev[MAXLEN];
    int i, len;

    i = 0;
    len = l;    /* duplicate length of initial array */

    if(s[l] == '\0')
    {
        --l;
    }

    while(i < len)
    {
        rev[i] = s[l];  /* reversing */
        ++i;
        --l;
    }

    i = 0;

    while (i < len) /* what i think is unnecessary */
    {
        s[i] = rev[i];  /* storing back to initial array */
        ++i;
    }
}
New contributor
Abhishek is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
  • \$\begingroup\$ Why you use while over for when you know exactly how many iterations do you need? I am saying about while(i < len). \$\endgroup\$ – ventaquil Jan 14 at 17:32
1
\$\begingroup\$

Standard C requires that main() return int:

int main(void)

There's no need (for 20 years now) to write all local declarations before the code. Introduce variables as you need them, so they can be initialised immediately:

{
    char line[MAXLEN];

    int i = 0;

There's no need for the extra storage rev in reverse(). We can reverse the string in place, by swapping first and last characters, then the second and second-last, and so on.

Use the Standard Library - it provides functions such as scanf(), getline(), strcpy() and strlen(). Don't write your own code to do these things - it makes your code much less clear, and causes readers to wonder what's being done that's different to the standard.

\$\endgroup\$
  • \$\begingroup\$ I was making a repo of all exercises in C language by K&R 2E and was thinking of keeping things simple according to the chapter and not using additional standard libraries. Can you actually answer how extra while loop can be eliminated ? \$\endgroup\$ – Abhishek Jan 14 at 12:10
  • \$\begingroup\$ As I said, reverse the string in-place, swapping pairs of characters until you're done. When you've removed the need for rev, you won't need that strcpy() loop. \$\endgroup\$ – Toby Speight Jan 14 at 12:30
  • \$\begingroup\$ can you provide a code snippet, my implementation is just printing palindrome strings ? \$\endgroup\$ – Abhishek Jan 14 at 13:02
  • 1
    \$\begingroup\$ No. This is Code Review, not a "write my program" service. \$\endgroup\$ – Toby Speight Jan 14 at 13:21
  • \$\begingroup\$ okay, thanks for your insight, appreciate it \$\endgroup\$ – Abhishek Jan 14 at 13:23
1
\$\begingroup\$

First off, the second loop should be a for loop:

for (i = 0; i < len; ++i) /* what i think is unnecessary */
{
    s[i] = rev[i];  /* storing back to initial array */
}

But to eliminate that, try:

while(i < l)
{
    char tmp = s[i]; /* swapping */
    s[i] = s[l]; /* swapping */
    s[l] = tmp; /* swapping */
    ++i;
    --l;
}

If there is a true middle character, you don't wind up moving it.

The check for the trailing NUL is also questionable. If you've been told the length, you should honor it.

\$\endgroup\$

Your Answer

Abhishek is a new contributor. Be nice, and check out our Code of Conduct.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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