4
\$\begingroup\$

The code performs bubble sort on the basis if any swaps has been performed in the iteration. I made it sort of independent of number of iterations as in any conventional bubble sorting code.

I went on pretty much with what I understood from the cs50 class. This mechanism looks more intuitive to me. What improvements I need to do on the design/style/method?

#include<stdio.h>

void printMyArray(int a[],int n);

//BUBBLE SORT
int main(){
    int a[] = {-1 , 2 , 0 , -3 , 5 , 1};
    int n = 6;
    for(;;){

       int swap = 0;
       int i = 0;

       for(; i < n-1; i++)
          if( a[i+1] <= a[i]){
              int temp = a[i+1];
              a[i+1] = a[i];
              a[i] = temp;
              swap = 1;
           }
       if(swap == 0)
           break;
    }
    printMyArray(a,6);

}
void printMyArray(int a[],int n){
   int i;
   for(i = 0; i < n ; i++)
     printf("%d\t",a[i]);
   printf("\n");
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ One obvious optimization: Don't use bubblesort! \$\endgroup\$ – Edward May 8 '16 at 22:05
  • \$\begingroup\$ I get the point but I am asking in terms of improving the above scenario. \$\endgroup\$ – piepi May 8 '16 at 22:11
6
\$\begingroup\$

Function

  1. Certainly no reason to swap when values are equal. In worse case of array {3,3,3,3}, leads to infinite loop.

    //if( a[i+1] <= a[i]){
    if(a[i+1] < a[i]) {
    
  2. Use size_t to index arrays.

    // void printMyArray(int a[],int n){
    void printMyArray(int a[], size_t n) {
    

Style

  1. Rather than int for Boolean variables use bool.

    #include <stdbool.h>
    
    // int swap = 0;
    bool swap = false;
    
  2. Recommend const when able. Adds clarity to the function and allows for optimizations with some compilers.

    // void printMyArray(int a[],int n){
    void printMyArray(const int a[], int n) {
    
  3. Minor: prefer int *a rather than int a[] as argument in printMyArray().

  4. More useful to separate main() from the code under test. Something like:

    int main(void) {
        int a[] = {-1 , 2 , 0 , -3 , 5 , 1};
        bubble_sort(a, sizeof a/ sizeof a[0]);
        printMyArray(a, sizeof a/ sizeof a[0]);
        return 0;     
    }
    
  5. Suggest fully {} blocks. Use Idiomatic for(), Avoid n-1, as size_t n may be 0.

       // int i = 0;
       // for(; i < n-1; i++)
    
       for(size_t i = 1; i < n; i++) {
         if(a[i] < a[i-1]) {
           int temp = a[i];
           a[i] = a[i-1];
           a[i-1] = temp;
           swap = 1;
         }
       }
    
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for pointing out the bug though. \$\endgroup\$ – piepi May 9 '16 at 1:05

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.