-3

Following algorithm works pretty fine in C#

    public int[] Sortieren(int[] array, int decide)
    {
        bool sorted;
        int temp;
        for (int i = 0; i < array.Length; i++)
        {
            do
            {
                sorted= true;
                for (int j = 0; j < array.Length - 1; j++)
                {
                    if (decide == 1)
                    {
                        if (array[j] < array[j + 1])
                        {
                            temp = array[j];
                            array[j] = array[j + 1];
                            array[j + 1] = temp;
                            sorted= false;
                        }
                    }else if (decide == 0)
                    {
                        if (array[j] > array[j + 1])
                        {
                            temp = array[j];
                            array[j] = array[j + 1];
                            array[j + 1] = temp;
                            sorted= false;
                        }
                    }
                    else
                    {
                        Console.WriteLine("Incorrect sorting parameter!");
                        break;
                    }
                }
            } while (!sorted);
        }
        return array;
    }

Same thing in C fails. I only get the first two numbers of the array being sorted. The rest of the numbers are same. So, this code also seems to change the array instead of only sorting it. Any ideas, where are the bugs?

#include <stdio.h>
#include<stdbool.h>
#define MAX 10

void main(void)
{
   int random_numbers[MAX],temp,Array_length;
   bool sorted;
   srand(time(NULL));
   for(int i=0;i<=MAX;i++){
        random_numbers[i]=rand()%1000;
   }
   Array_length=sizeof(random_numbers) / sizeof(int);
   printf("List of (unsorted) numbers:\n");
   for(int i=0;i<MAX;i++){
        if(i==MAX-1)
            printf("%i",random_numbers[i]);
        else
            printf("%i,",random_numbers[i]);
   }
   //Searching algorithm
   for(int i=0;i<Array_length;i++){
    do{
       sorted=true;
       for(int j=0;j<Array_length-1;j++){
        if(random_numbers[j]>random_numbers[j+1]){
            temp=random_numbers[j];
            random_numbers[j]==random_numbers[j+1];
            random_numbers[j+1]=temp;
            sorted=false;
        }
       }
    }while(!sorted);
   }
   printf("\n");
   for(int i=0;i<Array_length;i++){
        if(i==Array_length-1)
            printf("%i",random_numbers[i]);
        else
            printf("%i,",random_numbers[i]);
   }
}
Paul Floyd
  • 3,765
  • 5
  • 25
  • 37
tklustig
  • 347
  • 4
  • 19
  • 1
    don't use bubblesort. just don't – Mitch Wheat Apr 15 '18 at 06:59
  • 5
    What does this mean you think: `zufallszahlen[j]==zufallszahlen[j+1];`? – rene Apr 15 '18 at 07:00
  • 1
    Do you keep adding those +++ to overcome a quality warning or do you just like them to be there? Also I wouldn't use a code snippet to markup code that is not html or javascript. – rene Apr 15 '18 at 07:10
  • You might consider replacing the strings of '+' signs with details of what you found during your testing/debugging - that would be a lot more useful to those who may with to help and would avoid down/close votes. – Martin James Apr 15 '18 at 07:24
  • 1
    Bubblesort is never recommended. Please unlearn this algorithm. *"Even other О(n2) sorting algorithms, such as insertion sort, generally run faster than bubble sort, and are no more complex. Therefore, bubble sort is not a practical sorting algorithm."* – Antti Haapala Apr 15 '18 at 09:13
  • Took the liberty of submitting a translation of your variable names into english. Really recommend that you start using english only terms in your code. – Aziuth Apr 15 '18 at 09:26

1 Answers1

2

You have an error in your swap algorithm:

if (zufallszahlen[j] > zufallszahlen[j+1]) {
    temp = zufallszahlen[j];
    zufallszahlen[j] == zufallszahlen[j+1]; // here
    zufallszahlen[j+1] = temp;
    sortiert = false;
}

In the line after you assign to temp, your double equal sign results in a check for equality rather than an assignment. This is still legal code (== is an operator and and expressions that use them evaluate to something), and the expression will evaluate to either 1 or 0 depending on the truth value of the statement. Note that this is legal even though you're not using the expression, where normally a boolean value would presumably be used for control flow.

Note that this is true for other operators as well. For example, the = operator assigns the value on the right to the variable on the left, so hypothetically a mistake like if (x = 0) will mean this branch will never be called, since the x = 0 will evaluate to false every time, when you may have meant to branch when x == 0.

Also, why are you using a boolean value to check if the array is sorted? Bubble sort is a simple algorithm, so it should be trivial to implement, and by the definition of an algorithm, it's guaranteed to both finish and be correct. If you were trying to optimize for performance purposes, for example choosing between merge sort and insertion sort based on whether the data was already sorted then I would understand, but you're checking whether the data is sorted as you're sorting it, which doesn't really make sense, since the algorithm will tell you when it's sorted because it will finish. Adding the boolean checking only adds overhead and nets you nothing.

Also, note how in your C# implementation, you repeated the sort process. This is a good sign your design is wrong. You take in an integer as well as the actual int[] array in your C# code, and you use that integer to branch. Then, from what I can gather, you sort using either < or >, depending on the value passed in. I'm pretty confused by this, since either would work. You gain nothing from adding this functionality, so I'm confused as to why you added it in.

Also, why do you repeat the printf statements? Even doing if/else if I might understand. But you're doing if/else. This is logically equivalent to P V ~P and will always evaluate to true, so you might as well get rid of the if and the else and just have one printf statement.

Below is implementation of your Bubble Sort program, and I want to point out a few things. First, it's generally frowned upon to declare main as void (What should main() return in C and C++?).

I quickly want to also point out that even though we are declaring the maximum length of the array as a macro, all of the array functions I defined explicitly take a size_t size argument for referrential transparency.

Last but not least, I would recommend not declaring all your variables at the start of your program/functions. This is a more contested topic among developers, especially because it used to be required, since compilers needed to know exactly what variables needed to be allocated. As compilers got better and better, they could accept variable declarations within code (and could even optimize some variables away altogether), so some developers recommend declaring your variables when you need them, so that their declaration makes sense (i.e... you know you need them), and also to reduce code noise.

That being said, some developers do prefer declaring all their variables at the beginning of the program/function. You'll especially see this:

int i, j, k;

or some variation of that, because the developer pre-declared all of their loop counters. Again, I think it's just code noise, and when you work with C++ some of the language syntax itself is code noise in my opinion, but just be aware of this.

So for example, rather than declaring everything like this:

int zufallszahlen[MAX], temp, Array_length;

You would declare the variables like this:

int zufallszahlen[MAX];
int Array_length = sizeof (zufallszahlen) / sizeof (int);

The temp variable is then put off for as long as possible so that it's obvious when and were it's useful. In my implementation, you'll notice I declared it in the swap function.

For pedagogical purposes I would also like to add that you don't have to use a swap variable when sorting integers because you can do the following:

a = a + b;
b = a - b;
a = a - b;

I will say, however, that I believe the temporary swap variable makes the swap much more instantly familiar, so I would say leave it in, but this is my own personal preference.

I do recommend using size_t for Array_length, however, because that's the type that the sizeof operator returns. It makes sense, too, because the size of an array will not be negative.

Here are the include statements and functions. Remember that I do not include <stdbool.h> because the bool checking you were doing was doing nothing for the algorithm.

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

#define MAX 10

void PrintArray(int arr[], size_t n) {
    for (int i = 0; i < n; ++i) {
        printf("%d ", arr[i]);
    }

    printf("\n");
}

void PopulateArray(int arr[], size_t n) {
    for (int i = 0; i < n; ++i) {
        arr[i] = rand() % 1000 + 1;
    }
}

void BubbleSortArray(int arr[], size_t n) {
    for (int i = 0; i < n; ++i) {
        for (int j = 0; j < n - 1; ++j) {
            if (arr[j] > arr[j+1]) {
                int temp = arr[j+1];
                arr[j+1] = arr[j];
                arr[j] = temp;
            }
        }
    }
}

To implement the bubble sort algorithm, the only thing you have to do now is initialize the random number generator like you did, create your array and populate it, and finally sort the array.

int main()
{
    srand(time(NULL));

    int arr[MAX];
    size_t array_length = sizeof (arr) / sizeof (int);

    PopulateArray(arr, array_length);
    PrintArray(arr, array_length);
    BubbleSortArray(arr, array_length);
    PrintArray(arr, array_length);
}

I hope this helps, let me know if you have any questions.