-1

When I run this program in Dev C++, it executes until "enter the no of elements u want in an array" and when I give 5 numbers as input it crashes. Can anyone help me out? here is an implementation of quick sort using c

#include<stdio.h>
#include<stdlib.h>

void swap(int *a,int *b)//function to swap array
{
    int temp;
    temp=*a;
    *a=*b;
    *b=temp;

}

void quicksort(int arr[],int start,int end)//function for performig quicksort
{
    if(start>=end)
    {
        return;
    }
    else
    {
        int pindex= partition(arr,start,end);
        quicksort(arr,start,pindex-1);
        quicksort(arr,pindex+1,end);
    }
}

int partition(int arr[],int start,int end)//function to partition the array
{
    int i;
    int pivot=arr[end];//always making the last element in the array as pivot
    int pindex=arr[start];
    for(i=0;i<end;i++)
    {
        if(arr[i]<=pivot)
        {
            swap(&arr[i],&arr[pindex]);
        }
        swap(&arr[pindex],&arr[end]);
    }
}


void display(int arr[],int n)//function to display the sorted array
{
    int i=0;
    for(i=0;i<n;i++)
    {
        printf("%d",arr[i]);
    }
}

void main() // main function initializing here 
{
    int n,i,arr[20];
    printf("enter the no of elements u want in an array");
    scanf("%d",&n);
    for(i=0;i<n;i++)
    {
        scanf("%d",&arr[i]);
        printf("\n");
    }
    quicksort(arr,0,n);
    display(arr,n);
    getch();

}

This is my complete code of  quick sort program.i used different functions for different displaying output,swapping numbers partitioning etc.
Pritam Gogreen
  • 1
  • 1
  • 3
  • 2
    What is your question? Your code doesn't look like modern C to me, you have a function with no return type and a `main` that is `void`. Your compiler should warn you about them. – Jens Gustedt Aug 29 '15 at 16:31
  • 2
    `quicksort(int arr[],int start,int end)` --> `void quicksort(int arr[],int start,int end)`, `int pivot=arr[end];` --> `int pivot=arr[end - 1];`?, `swap(arr[pindex],arr[end]);` --> `swap(arr[pindex],arr[end - 1]);`? – Spikatrix Aug 29 '15 at 16:43
  • 3
    Note that you should declare the return type of `quicksort`; since it doesn't return a value, you should explicitly use `void quicksort(…)`. See also [What should `main()` return in C and C++](http://stackoverflow.com/questions/204476/what-should-main-return-in-c-and-c/18721336#18721336). – Jonathan Leffler Aug 29 '15 at 16:52
  • 2
    You have two calls to `swap` (at least): `swap(&arr[i],&arr[pindex]);` and `swap(arr[pindex],arr[end]);` They can't both be right. If you declared your functions before you called them, your compiler would tell you that you're calling `swap` incorrectly in the second line – you're passing integers instead of pointers. Whether that's the only cause of a crash is a separate issue, but it will certainly cause a crash. Turn on compiler warnings. Heed those warnings and fix the code before running it. And when testing sort code, print the data before the sort to make sure your input is OK. – Jonathan Leffler Aug 29 '15 at 16:56
  • 1
    the display function will output the integers, jammed together, on a single line. I.E. not very readable. suggest changing the format parameter to "%d\n" to print out the integers one per line – user3629249 Aug 29 '15 at 17:04
  • The debugger (`gdb`) should help you. You need to learn how to use the debugger (and to compile with all warnings & debug info, e.g. `gcc -Wall -Wextra -g`) – Basile Starynkevitch Aug 29 '15 at 17:08
  • 1
    @user3629249: I agree on the 'cramped together' comment. If the arrays being sorted are going to become large (say 20 elements or more), then packing more than one number per line becomes sensible. Maybe print up to 10 elements per line. And finish the output with a newline. I usually add a '`const char *tag`' argument to display functions which is then printed before the output; it can be used to identify 'before' and 'after', for example. – Jonathan Leffler Aug 29 '15 at 17:08
  • the array is defined with a max of 20 elements, what if the user enters 30 for the number of elements? Then the 21st through 30 element would be entered past the end of the array. This results in undefined behaviour and can lead to a seg fault event. Suggest changing arr[20] to 'int *arr;' on a separate line. then after the number of elements 'n' is entered, use something like: 'arr = malloc( n * sizeof(int) ); (of course, then check that 'arr' is not NULL) At the end of main(), call 'free(arr);' Note, for malloc and free, need the stdlib.h header file – user3629249 Aug 29 '15 at 17:12
  • you might want to (using paper and pencil) walk through the `partition()` function. It seems to be swapping the wrong elements – user3629249 Aug 29 '15 at 17:26

2 Answers2

2

the posted code does not even come close to compiling.

strongly suggest compiling with all warnings enabled

(for gcc at a minimum use: -Wall -Wextra -pedantic to enable warnings

Fix the warnings.

Then re-post the code

Just for your information:

  • use: int main( void ) or int main( int argc, char *argv[] )
  • place a prototype for each function, other than main(), at the beginning of the file, just after the #include statements
  • the conio.h header file is not portable (so should not be used) suggest using stdlib.h
  • any function that is not a void return type must have a return value; statement

  • the function: getch() is not portable (so should not be used) suggest using getchar()

  • in general, for code layout, use:

'#include' statements
blank line
'#define' statements
blank line
'static' file scope variables
blank line
prototypes for all functions except main()
blank line
'int main()' function
blank line
each sub function, separated by a blank line
user3629249
  • 16,402
  • 1
  • 16
  • 17
0

given this code:

scanf("%d",&n);
for(i=0;i<n;i++)
{
    scanf("%d",&arr[i]);
    printf("\n");
}

the user will see a prompt for the initial input, then only a blinking cursor.

A second prompt for the actual values, along with the directive to only enter one value per line would be good coding practice.

When calling scanf() (and family of functions) always check the returned value (not the parameter value) to assure the input operation was successful.

Suggest, in the existing prompt to the user, add the max value (20) and check in the code that the user entered value is less than 21.

Suggest, for the initial scanf() to change the format specifier to "%u" to assure the user cannot enter a negative value.

Suggest, checking the user did not enter 0 for the number of elements

Since the user will be entering values in any of several different ways, what if they entered:

5 1 2 3 4 5

The call to printf() in the middle of the input loop will result in 5 blank lines being inserted into the terminal. This would not be desirable.

Suggest removing the current printf() from the input loop

user3629249
  • 16,402
  • 1
  • 16
  • 17