1

I'm taking a C++ class and we've gotten to pointers. The assignment we've been given is to basically bubble sort an array that was read from a text file by passing pointers as parameters for various functions. I think I have a decent setup that outputs what I'm looking for, but for specific actions, I'm getting a zero as an element when there isn't one written to the array.

#include <iostream>
#include <fstream>

using namespace std;

int capacity;
int count;

int readData(int *&arr);
void swap(int *xp, int *yp);
void bsort(int *arr, int last);
void writeToConsole(int *arr, int last);
void bubble_sort(int *arr, int last, int(*ptr)(int, int));
int ascending(int a, int b);
int descending(int a, int b);

int main() {

    int *whatever = NULL;
    count = readData(whatever);
    cout << "Raw array data:" << endl;
    writeToConsole(whatever, capacity);
    cout << "After simple bubble sort:" << endl;
    bubble_sort(whatever, capacity, ascending);
    writeToConsole(whatever, capacity);
    cout << "Now descending:" << endl;
    bubble_sort(whatever, capacity, descending);
    writeToConsole(whatever, capacity);


    return 0;
}

int readData(int *&arr) {
    ifstream inputFile;
    inputFile.open("data.txt");
    if (!inputFile) {
        cout << "Error!";
    }
    inputFile >> capacity;
    arr = new int[capacity];
    for(int i = 0; i < capacity; i++){
        inputFile >> arr[i];
    }
    inputFile.close();
    return capacity;
}

void swap(int *xp, int *yp) {  
    int temp = *xp;  
    *xp = *yp;  
    *yp = temp;  
}

void bsort(int *arr, int last) {
   int i, j; 
   bool swapped; 
   for (i = 0; i < last + 1; i++) 
   { 
     swapped = false; 
     for (j = 0; j < last-i; j++) 
     { 
        if (arr[j] > arr[j+1]) 
        { 
           swap(arr[j], arr[j+1]); 
           swapped = true; 
        } 
     } 
     // IF no two elements were swapped by inner loop, then break 
     if (swapped == false) 
        break; 
   } 
} 

void writeToConsole(int *arr, int last) {

    cout << "[ ";
    for(int i = 0; i < last; i++){
        cout << arr[i] << " ";
    }
    cout << "]" << endl;
}

void bubble_sort(int *arr, int last, int(*ptr)(int, int)){
    int i, j; 
    bool swapped; 
    for (i = 0; i < last; i++) 
    { 
      swapped = false; 
      for (j = 0; j < last-i; j++) 
      {
         //Use the function pointer to determine which logic to use
         if (ptr(arr[j] , arr[j+1])) 
         { 
           swap(arr[j], arr[j+1]); 
           swapped = true; 
         } 
      }  

      // IF no two elements were swapped by inner loop, then break 
      if (swapped == false) 
        break; 
    } 
}

int ascending(int a, int b){
    return a > b;
}

int descending(int a, int b){
    return a < b;
}

My output looks like this:

Raw array data: [ 8 4 7 2 9 5 6 1 3 ] After simple bubble sort: [ 0 1 2 3 4 5 6 7 8 ] Now descending: [ 9 8 7 6 5 4 3 2 1 ]

Any ideas as to why my second sort is throwing in a zero? Thank you!

  • Time to learn how to use a debugger to step through your code statement by statement while monitoring variables and their values. – Some programmer dude Sep 18 '19 at 15:49
  • 2
    `arr[j+1]` (in `bubble_sort`) is undefined behavior, when `i == 0`, and `j == last - i - 1`, since it tries to access an element `arr[last]`, which is out of bounds. – Algirdas Preidžius Sep 18 '19 at 15:49
  • You should read [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). You defined a function called `swap`. C++ already contains [std::swap](https://en.cppreference.com/w/cpp/algorithm/swap). This can cause strange errors. I can't compile your code because of your global variable `count` and [std::count](https://en.cppreference.com/w/cpp/algorithm/count). The error message is `error: reference to 'count' is ambiguous`. – Thomas Sablik Sep 18 '19 at 15:55
  • You are actually not using your own `swap` function. You are using [std::swap](https://en.cppreference.com/w/cpp/algorithm/swap). You can check it if you add a line like `std::cout << "My Swap\n";` into your swap function. It's not called. Your swap function expects two pointers. `arr[j]` and `arr[j+1]` are integer values. You can't pass two integer values to your `swap` function. – Thomas Sablik Sep 18 '19 at 16:04
  • Here you can see it: https://wandbox.org/permlink/YvkNrUHsKjpX732O – Thomas Sablik Sep 18 '19 at 16:10
  • @AlgirdasPreidžius can you expand on that a bit? The way I'm understanding that is `arr[j+1]` basically equals `arr[j+1-1]` when `i == 0` ? – axionevolved Sep 18 '19 at 17:03
  • @axionevolved I thought, that I expanded enough. When `i == 0`, the loop condition is `j < last - i` === `j < last`. Hence the last possible value of `j` is `last - 1`. So, `j + 1`, given `j = last - 1` is `last + 1 - 1` === `last`. Which is out of bounds of array, since the last valid index is `last - 1`. – Algirdas Preidžius Sep 18 '19 at 17:07
  • In any case, why `j+1`? Looks like this was meant as `i`. – Jeffrey Sep 18 '19 at 17:57

1 Answers1

0

you have to do this change is in bubble_sort function

for (j = 1; j < last - i; j++)
        {
            //Use the function pointer to determine which logic to use
            if (ptr(arr[j-1], arr[j]))
            {
                swap(arr[j-1], arr[j]);
                swapped = true;
            }
        }
Vahid Bahramian
  • 134
  • 1
  • 7