###Follow standard conventions
C++ standard algorithms follow a convention of using iterators to define ranges on which to work. Rather than passing a container pass two iterators to define a range.
Secondly you should be able to sort anything (that is comparable (defines operator<)) not just integers. If you want to be flashy allow a function to be passed that knows how to compare objects (but that is a phase two just assume that you can compare any type with operator< and the compiler will let you know when that is not true).
// Rather than this:
void QuickSort(vector<uint>& inp, uint l, uint r, uint& numOfCmp)
// Define it like this:
template<typename I>
void QuickSort(I begin, I end);
###Dont use this:
using namespace std;
see: Why is "using namespace std" in C++ considered bad practice? Why is "using namespace std" in C++ considered bad practice? and Answer 2 Answer 2
###There is already a std::swap
auto swap = [&inp](uint a, uint b)
{
uint buf = inp[a];
inp[a] = inp[b];
inp[b] = buf;
};
Just use the standard swap std::swap() there is even one that works on iterators std::iter_swap().
###Not convinced that algorithm is correct.
Normally when I see the pivot/partition I see the code scanning from left and right towards the middle. Here I see a scan from left to right only. But it seems to work.
//numOfCmp += r - l; // we can use this, but ++numOfCmp just before
// comparison is more clear
uint i = l + 1;
uint j = l + 1;
uint p = inp[l];
for (; j <= r; j++)
{
++numOfCmp;
if (inp[j] <= p)
{
if (i != j)
swap(i, j);
i++;
}
}
###Follow standard conventions
C++ standard algorithms follow a convention of using iterators to define ranges on which to work. Rather than passing a container pass two iterators to define a range.
Secondly you should be able to sort anything (that is comparable (defines operator<)) not just integers. If you want to be flashy allow a function to be passed that knows how to compare objects (but that is a phase two just assume that you can compare any type with operator< and the compiler will let you know when that is not true).
// Rather than this:
void QuickSort(vector<uint>& inp, uint l, uint r, uint& numOfCmp)
// Define it like this:
template<typename I>
void QuickSort(I begin, I end);
###Dont use this:
using namespace std;
see: Why is "using namespace std" in C++ considered bad practice? and Answer 2
###There is already a std::swap
auto swap = [&inp](uint a, uint b)
{
uint buf = inp[a];
inp[a] = inp[b];
inp[b] = buf;
};
Just use the standard swap std::swap() there is even one that works on iterators std::iter_swap().
###Not convinced that algorithm is correct.
Normally when I see the pivot/partition I see the code scanning from left and right towards the middle. Here I see a scan from left to right only. But it seems to work.
//numOfCmp += r - l; // we can use this, but ++numOfCmp just before
// comparison is more clear
uint i = l + 1;
uint j = l + 1;
uint p = inp[l];
for (; j <= r; j++)
{
++numOfCmp;
if (inp[j] <= p)
{
if (i != j)
swap(i, j);
i++;
}
}
###Follow standard conventions
C++ standard algorithms follow a convention of using iterators to define ranges on which to work. Rather than passing a container pass two iterators to define a range.
Secondly you should be able to sort anything (that is comparable (defines operator<)) not just integers. If you want to be flashy allow a function to be passed that knows how to compare objects (but that is a phase two just assume that you can compare any type with operator< and the compiler will let you know when that is not true).
// Rather than this:
void QuickSort(vector<uint>& inp, uint l, uint r, uint& numOfCmp)
// Define it like this:
template<typename I>
void QuickSort(I begin, I end);
###Dont use this:
using namespace std;
see: Why is "using namespace std" in C++ considered bad practice? and Answer 2
###There is already a std::swap
auto swap = [&inp](uint a, uint b)
{
uint buf = inp[a];
inp[a] = inp[b];
inp[b] = buf;
};
Just use the standard swap std::swap() there is even one that works on iterators std::iter_swap().
###Not convinced that algorithm is correct.
Normally when I see the pivot/partition I see the code scanning from left and right towards the middle. Here I see a scan from left to right only. But it seems to work.
//numOfCmp += r - l; // we can use this, but ++numOfCmp just before
// comparison is more clear
uint i = l + 1;
uint j = l + 1;
uint p = inp[l];
for (; j <= r; j++)
{
++numOfCmp;
if (inp[j] <= p)
{
if (i != j)
swap(i, j);
i++;
}
}
###Follow standard conventions
C++ standard algorithms follow a convention of using iterators to define ranges on which to work. Rather than passing a container pass two iterators to define a range.
Secondly you should be able to sort anything (that is comparable (defines operator<)) not just integers. If you want to be flashy allow a function to be passed that knows how to compare objects (but that is a phase two just assume that you can compare any type with operator< and the compiler will let you know when that is not true).
// Rather than this:
void QuickSort(vector<uint>& inp, uint l, uint r, uint& numOfCmp)
// Define it like this:
template<typename I>
void QuickSort(I begin, I end);
###Dont use this:
using namespace std;
see: Why is "using namespace std" in C++ considered bad practice? and Answer 2
###There is already a std::swap
auto swap = [&inp](uint a, uint b)
{
uint buf = inp[a];
inp[a] = inp[b];
inp[b] = buf;
};
Just use the standard swap std::swap() there is even one that works on iterators std::iter_swap().
###Not convinced that algorithm is correct.
Normally when I see the pivot/partition I see the code scanning from left and right towards the middle. Here I see a scan from left to right only. But it seems to work.
//numOfCmp += r - l; // we can use this, but ++numOfCmp just before
// comparison is more clear
uint i = l + 1;
uint j = l + 1;
uint p = inp[l];
for (; j <= r; j++)
{
++numOfCmp;
if (inp[j] <= p)
{
if (i != j)
swap(i, j);
i++;
}
}