2
\$\begingroup\$
//Program To Input Values Into An Array And Sort Them In Ascending Order
#include<stdio.h>
//Function To Print The Array Values
void print(int tab[],int n)
{
    int i;
    printf("The Values Are : \n");
    for(i=0;i<n;i++)
    {
        printf("%d ",tab[i]);
    }
        printf("\n");//Line Jumping For Better Output
}
//Function To Swap The Values In The Array
void swap(int* x,int* y)
{
    int temp=*x;
    *x=*y;
    *y=temp;
}
//Function To Sort The Values In The Array By Using Previous Function
void sort(int tab[],int n)
{
    int i,j;
    for(i=0;i<n-1;i++)
    {
        for(j=i+1;j<n;j++)
        {
            if(tab[i]>tab[j])
            {
                swap(&tab[i],&tab[j]);//Swaps The Value Of Index i With The Value Of Index j In The Array
            }
        }
    }
}
//Main Code
int main()
{
    int i,n,tab[100];
    printf("Give The Number Of Elements In The Array:");
    scanf("%d",&n);
    while(n<=0)
    {
    printf("The Number Of Elements MUST Be Superior To 0!\n");
    printf("Give The Number Of Elements In The Array:");
    scanf("%d",&n);
    }
    printf("Insert %d Values :\n",n);
    for(i=0;i<n;i++)
    {
        scanf("%d",&tab[i]);
    }
    print(tab,n);//Prints The Values In The Array
    sort(tab,n);//Sorts The Values In The Array
    printf("SORTING....\n");
    print(tab,n);//Prints The Values In The Array
    return 0;
}
\$\endgroup\$
1
\$\begingroup\$

General Observations

Good job creating functions and the naming of the functions. The naming of the variables could probably be improved, for instance n could be renamed to item_count or array_element_count.

The indentation is inconsistent. This could lead issues during maintenance of the code.

Missing Function

As stated above you did a pretty good job of creating functions, but you missed one, the input of the array should be a function as well.

Logical Order of Printing

Put printf("SORTING...\n"); before the call to the sort() function.

Comments

Since you did a good job of naming the functions most of your comments are unnecessary. For instance //Swaps The Value Of Index i With The Value Of Index j In The Array really doesn't add anything.

Arguments for Swap Function

Rather than passing in 2 pointers to values and swapping the values I might pass in the array and the 2 indexes and use array notation in the swap function to relocate the values.

Missing Error Check

The code to input the array properly checks to see that the value of the size of the array is greater than 0, but it doesn't check that the size of the array is less than than the maximum size.

\$\endgroup\$
1
  • \$\begingroup\$ I would prefer void swap(int* x, int* y) or more general void swap(void* x, void* y, size_t n). As void swap(int* a, int x, int y) is too specified and not quite useful. (Just my prefer.) \$\endgroup\$ – tsh Feb 23 at 10:30
1
\$\begingroup\$

What you have implemented is called Bubble Sort Insertion Sort (If I've read it correctly); it's worth mentioning that in the comment of sort() to help readers. Otherwise, they may easily misunderstand the algorithm, as I did!


int main()

Not bad, but we can be a little more specific by showing that the main() takes no arguments:

int main(void)

We have a problem here:

int n;
scanf("%d",&n);
while(n<=0)

If scanf() is unable to convert the input as a decimal (e.g. if user writes four as input), then n will be uninitialized. That's bad, as it means your program has Undefined Behaviour, which in turn means it could do anything at all.

However, we are able to detect that. scanf() returns the number of successful conversions it managed, so we just need to use that return value to determine whether n holds a valid value:

if (scanf("%d", &n) != 1) {
    fputs("Not a valid number!\n", stderr);
    return 1;
}

We could avoid worrying about negative n by using an unsigned type (unsigned int, probably).

It's a design decision, but I think you should allow a zero-length (empty) array as input. It's really easy to sort! And with unsigned n, means that all values are valid.

Obviously the other scanf() needs its return value checking, too.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ It's neither bubble sort nor insertion sort but rather an overly eager version of selection sort. (The inner loop puts the smallest remaining value at tab[i], but always swaps immediately instead only finding the smallest remaining value and then swapping only once.) \$\endgroup\$ – superb rain Feb 25 at 14:32
1
\$\begingroup\$

Tidy up printing

White-space lurking between the last number and the '\n' is a surprising occurrence and in my experience a source of annoying issues with cut/paste of output.

const *

Use const for wider function usage. Function can now also get called with a const array.

Parameter order

Consider order of parameters in function ... size ... before the array

Consider size_t

int may be too small for large arrays. Could use size_t.


Put this together

void print(size_t n, const int tab[]) {
  printf("The Values Are :\n");  // No space before \n
  const char *sep = "";
  for (size_t i=0; i<n; i++) {
    printf("%s%d ", sep, tab[i]); // Space not after the number.
    sep = " ";
  }
  printf("\n");
}
\$\endgroup\$

Your Answer

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.