0
int arr[] = {7,4,10,8,3,1};
int size = sizeof(arr) / sizeof(arr[0]);

for(int i = 0; i<size-1; i++){
    
    int temp = arr[i];
    
    for(int j = i+1; j < size; j++){
        
        if(arr[j] < temp){
            temp = arr[j];
        }
        
    }
    
    swap(temp, arr[i]);
}

I am trying to apply the selection sort algorithm on the given array, but the output I am getting is only [1,1,1,1,1,1], I am finding the minimum element through the inner loop, Ican't figure out what is going wrong?

Alex
  • 81
  • 1
  • 6
  • Think about what `swap(temp, array[i]);` does. It's essentially `array[i] = temp;` since `temp` goes out of scope right afterwards – UnholySheep Aug 21 '20 at 22:24
  • IMHO, don't state the capacity of an array when you are defining the contents. Let the compiler figure it out. This way, you don't over-allocate space. Example: `const int arr[] = {7,4,10,8,3,1};` – Thomas Matthews Aug 21 '20 at 22:26
  • Where is `array` defined? – Thomas Matthews Aug 21 '20 at 22:27
  • @ThomasMatthews and then do `const auto size = sizeof(arr) / sizeof(*arr);` so we can use that in a loop condition? What's the point? – Fureeish Aug 21 '20 at 22:27
  • When using a number as the capacity, you now have to maintain a named identifier. IMHO, letting the compiler calculate the capacity means less defects due to the quantity of the contents mismatching with the stated capacity. – Thomas Matthews Aug 21 '20 at 22:30
  • 2
    It's 2020. Can we all agree to use `std::size`? – user4581301 Aug 21 '20 at 22:52
  • @user4581301 yes and no, lol. Some "big" players on commercial compiler arena still debate or don\'t support it. It was offered to be added to vs2019, not sure if it was. Some don't support `std::ssize()`. But I tend to work around it by offering own implementation. – Swift - Friday Pie Aug 22 '20 at 07:55
  • @Swift-FridayPie lol, I was just about to write an answer with `std::size` but I guess with your knowledge maybe it might not be helpful for the op. – Geno C Aug 22 '20 at 08:00
  • @GenoC Do it! So he'll know the dark side of `using namespace` or taking names of standard components (there I see a variable named `size` :P) – Swift - Friday Pie Aug 22 '20 at 08:10

5 Answers5

0

Slightly modified your code;

You need to pass reference(address) to both elements to take place of swapping contents

int arr[] = { 7, 1, 10, 8, 3, 11, 0, 12, 5, 8 };
int size = sizeof(arr) / sizeof(arr[0]);

for(int i = 0; i < size; i++)
{
   auto temp = std::min_element( arr + i, arr + size );

   std::swap( arr[i], *temp );      
}

You have to add algorithm header to use std::min_element

pvc
  • 1,070
  • 1
  • 9
  • 14
0
int arr[] = {7,4,10,8,3,1};
int size = sizeof(arr) / sizeof(arr[0]);

for(int i = 0; i<size-1; i++){
    
    int temp = arr[i];
    int pos = i;
    
    for(int j = i+1; j < size; j++){
        
        if(arr[j] < temp){
            temp = arr[j];
            pos = j;
        }
        
    }
    
    if(pos != i)
        std::swap(arr[pos], arr[i]);
}

This should work.

Harry
  • 2,177
  • 1
  • 19
  • 33
0

It is suggested not to use using namespace std;. There is a plethora of reasons why you should not; that I will not mention.

By the way I tried to keep some of your variables the same but to be honest, I didn't. It is better to create variable names that explain what the code is doing. It makes your code a lot more legible and readable.

So opt out of one letter variables. It is fine in for loops, however this is a special case.

Now, here is another alternative suggested by @user4581301 & @Swift -Friday Pie. This method is using std::size using c++17.

For example:

#include <iostream>
#include <utility> // to use the swap() function.
#include <iterator> // to use std::size() function.


int main()
{
    int arr[] = { 7,4,10,8,3,1 };
    // This --> int size = sizeof(arr) / sizeof(arr[0]); is archaic.

    const int length = static_cast<int>(std::size(arr)); // Call this something other than "size"; you can run into issues. 
   // We use static_cast<int>  as a implicit conversion, and the obvious std::size(arr)).
   
    

   // Going through the elements

    for (int StartOfIndex = 0; StartOfIndex < length - 1; ++StartOfIndex)
    {
        // smallest is the index of the smallest element we’ve encountered this iteration

        int smallest = StartOfIndex;

        // Looking for a smaller element..
        for (int current = StartOfIndex + 1; current < length; ++current)
        {
            // if we found an element smaller than our last; take note.
            if (arr[current] < arr[smallest])

                smallest = current;
        }

        // swap StartOfIndex with smallest.
        std::swap(arr[StartOfIndex], arr[smallest]);
    }

    //Prints array.
    for (int index = 0; index < length; ++index)
        std::cout << arr[index] << " ";

    std::cout << "\n";

    return 0;
}

Output: 1 3 4 7 8 10

Geno C
  • 1,401
  • 3
  • 11
  • 26
-1
void swap(int *xp, int *yp)  
{  
    int temp = *xp;  
    *xp = *yp;  
    *yp = temp;  
}  
  
void selectionSort(int arr[], int n)  
{  
    int i, j, min_idx;  
  
    // One by one move boundary of unsorted subarray  
    for (i = 0; i < n-1; i++)  
    {  
        // Find the minimum element in unsorted array  
        min_idx = i;  
        for (j = i+1; j < n; j++)  
        if (arr[j] < arr[min_idx])  
            min_idx = j;  
  
        // Swap the found minimum element with the first element  
        swap(&arr[min_idx], &arr[i]);  
    }  
}  


selectionSort(arr,size);

This should work.

ismailenes
  • 15
  • 1
  • 7
  • 1
    Avoid rolling your own `sort`, `std::sort` has pretty much everything built in covered already, and if you have to, prefer to pass by reference. – user4581301 Aug 21 '20 at 22:50
-1

The first mistake you made in writing for loop's condition, don't use swap(temp, array[i]); yet try to get the basics first.

#include <iostream>

using namespace std;

int findsmall(int arr[], int i, int size){
    int s, pos, j;
    s = arr[i];
    pos = i;
    for(j = i+1; j < size; j++){
        if(arr[j] < s){
            s = arr[j];
            pos = j;
        }
    }
    return pos;
}

int main() {
    
    int arr[] = {7,4,10,8,3,1};
    int size = sizeof(arr) / sizeof(arr[0]);
    int smallnum;
    int temp;
    int count = 0;
    
    cout << "Original array: ";
    
    for(int i = 0; i < size; i++){
        if(i < size - 1){
        cout << arr[i] << ", ";}
        else{
            cout << arr[i];
        }
    }
    
    cout << endl;
    
    for(int i = 0; i < size; i++){
        smallnum = findsmall(arr,i, size);
        temp = arr[i];
        arr[i] = arr[smallnum];
        arr[smallnum] = temp;
        count++;
    }
    
    
    
    cout << "Sorted array: ";
    
    for(int i = 0; i < size; i++){
        if(i < size - 1){
        cout << arr[i] << ", ";}
        else{
            cout << arr[i];
        }
    }
    
    cout << endl;
    
    return 0;
}
Loppy2323
  • 82
  • 6