I am writing code for selection sort in c . It gives no error when i compile it with the command g main.cpp -o main
in powershell but when i run the code with ./main
, it don't show anything. I tried with hello world program and it worked. I don't know why the selection sort code not working.
Here Is the code of Selection sort
#include<iostream>
using namespace std;
int main()
{
int n, a[n];
cout << "Enter the size of the array = ";
cin >> n;
cout << "Enter the numbers :" << endl;
for (int i = 0; i < n; i )
{
cin >> a[i];
}
for (int i = 0; i < n-1; i )
{
for (int j = i 1; j < n; j )
{
if (a[i] > a[j])
{
int temp = a[i];
a[i] = a[j];
a[j] = temp;
}
}
}
for (int b=0; b<n; b )
{
cout<<a[b];
}
return 0;
}
CodePudding user response:
Always initialize variables. A declaration without initialization is a code smell, eg this one:
int n;
The issue immediately follows, because
int n, a[n];
uses n
uninitialized. Compilers do a pretty good job at warning about this: https://godbolt.org/z/ErPY3x936. Before you initialize n
, it has an indeterminate value. Using its value leads to undefined behavior. Further, a[n]
is a variable length array (unless n
is a constant expression), which is not part of standard C . When you need an array whose size is only known at runtime, you can use a std::vector
:
int n = 0;
cout << "Enter the size of the array = ";
cin >> n;
std::vector<int> arr(n);
CodePudding user response:
You are trying to use a variable length array
int n, a[n];
Variable length arrays is nit a standard C feature. So you should avoid to use them. Instead use the standard container std::vector<int>
.
Moreover the variable n
is not initialized. So the declaration of the variable length array invokes undefined behavior.
Within the for loops where you are sorting the array there are too many swaps of elements of the array.
The selection sort algorithm assumes that a selected element in an array is swapped at most one time.
And there is the standard function std::swap
that can be used instead of manually swapping elements.
Your program can look the following way
#include <iostream>
#include <utility>
#include <vector>
int main()
{
size_t n = 0;
std::cout << "Enter the size of the array (0 - exit): ";
std::cin >> n;
if ( n )
{
std::vector<int> v( n );
std::cout << "Enter the numbers :" << std::endl;
for ( auto &item : v )
{
std::cin >> item;
}
for ( std::vector<int>::size_type i = 0; i < v.size(); i )
{
auto min = i;
for ( auto j = i 1; j < v.size(); j )
{
if ( v[j] < v[min] ) min = j;
}
if ( min != i ) std::swap( v[min], v[i] );
}
for ( const auto &item : v )
{
std::cout << item << ' ';
}
std::cout << std::endl;
}
return 0;
}.
CodePudding user response:
There are 2 problems in your program.
Mistake 1
In C the size of an array must be a compile time constant. So take for example,
int n = 10;
int arr[n] ; //INCORRECT because n is not a constant expression
The correct way to write the above would be:
const int n = 10;
int arr[n]; //CORRECT
Mistake 2
You're using an uninitialized variable which leads to undefined behavior. In particular when you wrote:
int n, a[n]; //here variable n is uninitialized and holds garbage value
In the above statement, you are creating an int
named n
but since you have not explicitly initialized it, it holds a garbage value.
Next, you're using that garbage value as the size of the array a
. But note that using uninitialized variable results in undefined behavior.
Undefined behavior means anything1 can happen including but not limited to the program giving your expected output. But never rely(or make conclusions based) on the output of a program that has undefined behavior.
This is why it is advised that
always initialize built in types in local/block scope.
Solution
A better way would be to use std::vector
as shown below.
#include <iostream>
#include <vector>
int main()
{
int n = 0; //always initialize built in types in local/block scope
std::cout<<"Enter size: "<<std::endl;
std::cin >> n;
//create a vector of size n
std::vector<int> a(n);
//iterate and ask for input
for(int i = 0; i < a.size(); i)
{
std::cout<<"Enter element: "<<std::endl;
std::cin >> a[i];
}
for (int i = 0; i < a.size() - 1; i)
{
int index = i;
for (int j = i 1; j < a.size(); j ) {
if (a[j] < a[index])
index = j;
}
int temp = a[index];
a[index] = a[i];
a[i] = temp;
}
std::cout<<"The elements of the vector are:"<<std::endl;
//print the element of the vector
for(const int& elem: a)
{
std::cout<<elem<<std::endl;
}
return 0;
}
The output of the program can be seen here.
1For a more technically accurate definition of undefined behavior see this where it is mentioned that: there are no restrictions on the behavior of the program.