3
\$\begingroup\$

I am a Java programmer who has C this semester. Needless to say, my writing style is heavily influenced by Java, which is probably not a great thing. I'd be grateful for a few nudges in the right direction. Here's the code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define LEN 100
#define WID 80

void sort(char s[][WID], int n);
void display(char s[][WID], int n);

int main()
{
    int n, i;
    char s[LEN][WID];
    printf("Enter number of strings : ");
    scanf("%d", &n);
    printf("Enter strings one by one :\n");
    for (i = 0; i < n; i++) {
        printf("%d : ", i + 1);
        scanf(" %s", s[i]);
    }

    printf("The initial elements are:\n");
    display(s, n);
    sort(s, n);
    printf("The elements after sorting are:\n");
    display(s, n);

    return 0;
}

void sort(char s[][WID], int n)
{
    int i, item;
    char elem[WID];

    for (item = 1; item < n; item++) {
        strcpy(elem, s[item]);
        for (i = item; strcmpi(s[i - 1], elem) > 0 && i > 0; i--) {
            //strcpy(s[i], s[i - 1]);
        }
        memcpy(&s[i + 1], &s[i], (item - i) * WID);
        strcpy(s[i], elem);
    }
}

void display(char s[][WID], int n)
{
    int i;
    printf("\n\n");
    for (i = 0; i < n; i++) {
        printf("%s ", s[i]);
    }
    printf("\n\n");
}

If you see something blasphemous, don't hesitate to call it out.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  1. Never read strings with scanf like this:

    scanf(" %s", s[i]);
    

    This allows the user of the program to enter arbitrarily long text which can overwrite all of the programs memory. There are several options of how to do this safely, one would be to use fgets which allows to specify the maximum length the input buffer can store.
    Make sure the input is terminated with '\0' if you do this.

  2. Same goes for the number of strings to read: You never check the counter against LEN.

  3. In sort elem is used as temporary storage, so a better name would be temp.

  4. Last but not least: You perform an awful lot of copying. This is totally unnecessary - you should really read up about pointers in C (they are somewhat similar to references in Java). Allocate the input strings dynamically and then just swap pointers instead of copying all those strings around.
    You won't be able to avoid learning about memory management and pointers in C so might as well dig into it.

\$\endgroup\$
1
  • \$\begingroup\$ I've posted a follow-up here. Have a look. :-) \$\endgroup\$ Commented Aug 14, 2017 at 16:14

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.