2

I am trying to practice with C by making a bubble sort program. The problem until now seems to be that the for loop that is giving values to the cells of the array is stuck after the condition is no longer fulfilled but it doesn't seem to be executing the commands in the loop. I don't know what is happening exactly and I have added some extra lines to see what is happening an these were my conclusions. Here is the code:

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

void swap(int *x, int *y)
{
    int temp = *x;
    *x = *y;
    *y = temp;
}

int *sort(int *array)
{
    int finish = 1;
    while (finish = 1)
    {
        finish = 0;
        for (int i = 0; i <= sizeof(array); i++)
        {
            if ((array + i) > (array + i + 1))
            {
                swap(array + i, array + i + 1);
                finish = 1;
            }
        }
    }
    return array;
}
int main()
{
    int s, res;
    printf("Give me the size of the array being sorted(larger than 1) : ");
    do
    {
        res = scanf("%d", &s);
        if (res != 1)
        {
            printf("Wrong Input!\n");
            exit(1);
        }
        if (s < 2)
            printf("Only numbers equal or larger than 2\n");

    } while (s < 2);
    int array[s];
    for (int i = 0; i < s; i += 1)
    {
        scanf("%d", array + i);
        printf("%d %d %d\n\n", *(array + i), i, i < s); // I used this to check if my values were ok
    }
    printf("end of reading the array"); //I added this line to see if I would exit the for loop. I am not seeing this message
    sort(array);
    printf("\n");
    for (int i = 0; i < sizeof(array); i++)
        printf("%d\n\n", array + i);
    printf("Array has been sorted! Have a nice day!\n\n************************************************************");
    return 0;
}
Improve this question
10
  • 2
    sizeof(array) won't do what you think it does. Look up "array to pointer decay". Commented Nov 14, 2018 at 12:14
  • 4
    Also while (finish = 1)-->while (finish == 1) Commented Nov 14, 2018 at 12:15
  • 2
    @Swordfish I thought int array[s]; should be an array....right? Commented Nov 14, 2018 at 12:16
  • 1
    Please always enable warnings in your compiler. The line while (finish = 1) should cause some warning about an assignment in a condition. Commented Nov 14, 2018 at 12:18
  • 1
    sizeof(array) is misused both times. In the sort() function, sizeof(array) returns the number of bytes that make up the pointer int *array. In main, sizeof(array) returns the number of bytes in the array int array[s]; and not the number of elements in the array. Commented Nov 14, 2018 at 12:24

3 Answers 3

Reset to default
3

See the annotations in the code:

#include <stddef.h>  // size_t  1)
#include <stdio.h>
#include <stdlib.h>

void swap(int *x, int *y)
{
    int temp = *x;
    *x = *y;
    *y = temp;
}

int *sort(int *array, size_t size)  // needs an extra parameter to know the size of the array
{
    int finish = 1;
    while (finish /* = 1 * you don't want assignment, you want comparison: */ == 1)
    {
        finish = 0;
        for (int i = 0; i /* <= sizeof(array) */ < size - 1; i++)  // i should be of type size_t
        {
            // if ((array + i) > (array + i + 1)) you are not dereferencing:
            if(array[i] > array[i + 1]) 
            {
                // swap(array + i, array + i + 1);  // easier to read imho:
                swap(&array[i], &array[i + 1]);
                finish = 1;
            }
        }
    }
    return array;  // why does this function return anything? it is never used.
}
int main()
{
    int s; /* , res;  no need for an extra variable res */
    printf("Give me the size of the array being sorted(larger than 1) : ");
    do
    {
        // res = scanf("%d", &s);
        // if (res != 1)
        if (scanf("%d", &s) != 1)
        {
            printf("Wrong Input!\n");
            // exit(1);  // should be EXIT_FAILURE. Use return instead of exit() when in main().
            return EXIT_FAILURE;
        }
        if (s < 2)
            printf("Only numbers equal or larger than 2\n");

    } while (s < 2);
    int array[s];
    for (int i = 0; i < s; /* i += 1* idiomatic: */ ++i)  // size_t would be the correct type for s and i.
    {
        scanf("%d", /* array + i  use indexes: */ &array[i]);
        printf("%d %d %d\n\n", array[i], i, i < s);  // again: indexes. i < s is allready ensured by the condition of the for-loop
    }
    printf("end of reading the array");
    // sort(array);  // sort will have no idea about the size of array use
    sort(array, s); // instead.

    printf("\n");
    for (int i = 0; i < /* sizeof(array) 2) */ s; i++)
        printf("%d\n\n", /* array + i * again you don't dereference */ array[i]);
    printf("Array has been sorted! Have a nice day!\n\n************************************************************");
    return 0;
}

1) size_t is the type that is guaranteed to be big enough to hold all sizes of objects in memory and indexes into them. The conversion specifier for scanf() is "%zu".

2) sizeof(array) in main() will yield the number of bytes in array, but you want the number of elements so you'd have to use sizeof(array) / sizeof(*array). But thats not needed since you already know its size. It is s.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

You cannot imagine how helpful this is thank you so much!! I learned a lot from this and you expanded my understanding of C thank you!
"You cannot imagine how helpful" Yes, i can. i learned C++ the very hard way without internet access and q+a sites or forums using Borland C++ 3.1 on DOS 6.22^^
i +1 < size handles the corner case of size == 0. It is better than i < size - 1 which will iterate for a long time as it is effectively i < SIZE_MAX.
0

This line

printf("end of reading the array");

has no line feed at the end of the string. This is a problem because printf is part of the family of functions called "buffered IO". The C library maintains a buffer of the things you want to print and only sends them to the terminal if the buffer gets full or it encounters \n in the stream of characters. You will not see, end of reading the array on your screen until after you have printed a line feed. You only do this after calling sort(). So all you know is your program is getting into an infinite loop at some point before the end of sort.

So there are actually three loops that could be infinite: the for loop you identified, the while loop in sort and the for loop inside the while loop. As the other answers point out, you have made the classic mistake of using assignment in the while conditional 

while (finish = 1)
//            ^ not enough equals signs

Unless your C compiler is really old, it is probably outputting a warning on that line. You should heed warnings.

Also, you should learn to use a debugger sooner rather than later. Believe me, it will save you a lot of time finding bugs.

Improve this answer

Comments

-1

In the sort function sizeof(array) returns the size of the pointer. (you can check it by yourself using printf("%d", sizeof(array).

The solution is to change your function to:

int sort(int* array, size_t size) { ... }

and call it with the correct array size:

sort(array, s);
Improve this answer

5 Comments

"returns the same as sizeof(int)" nope.
"printf("%s", sizeof(array))" nope, nope.
Does the sizeof not return the size of the pointer type? That was always my understanding of it.
Ok, I should have tested it myself before answering, it returns the size of the pointer type.
Should be using %zu not %d for printing out the sizeof something

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.