HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Quicksort application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
applicationquicksortstackoverflow

Problem

What do you think about this code?

#include 
using std::cout;
using std::endl;

#include 
using std::vector;

#include 
#include 

template 
vector  concatenate( vector  _a_, T piv, vector  _b_ )
{
  vector  v_list;
  v_list.insert( v_list.begin(), _a_.begin(), _a_.end() );
  v_list.push_back( piv );
  v_list.insert( v_list.end(), _b_.begin(), _b_.end() );
  return v_list;
}

template  
vector quick( vector vec )
{
  vector v_less, v_greater, v_list;
  vector aux1, aux2;
  typename vector ::const_iterator iter_Const;
  if( vec.size()  lista;
  srand( 1 );
  cout  :: const_iterator it;
  for( it = lista.begin(); it != lista.end(); ++it )
    cout << *it << " | ";
  cout << endl;
}

Solution

If this is not just a learning exercise, I have to point out that C++ already has a sorting function in its standard library and in real code you should use that. So for the rest of this answer I'm going to assume that this is in fact just a learning exercise.

Now first a note on the algorithm: Usually quicksort is implemented in-place for better performance. Even if you want a sorting function which does not modify its argument, it's quicker to just copy the argument and then run an in-place quicksort on the argument.

Further you're creating a lot more copies of your vectors than you need to, even when using this choice of algorithm: Every time you pass a vector by value, it is copied. Every time you pass a vector, which you do not want to modify, you should rather pass it by const reference, so as not to create an unnecessary copy.

On the same note concatenate also creates an unnecessary copy by returning the result vector by value. This can be avoided by passing an empty vector by non-const reference and filling that instead of using a return value.

Lastly there is a bit of code duplication in your main function: You're printing a vector separated by pipes twice - once during its creation and once after sorting. I would define a helper function print_vector, which prints a vector separated by pipes, and call that twice: once after creating the vector and once after sorting it. This way the first printing will have to happen after the vector creation, not during, but that's okay as doing it during the creation gives very little performance benefit (if any) and having the creation be a separate step from printing the vector improves code cleanliness.

Context

StackExchange Code Review Q#1259, answer score: 3

Revisions (0)

No revisions yet.