Skip to main content
added 786 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238
  • The last three libraries should be removed as they're not being used. There's no need to keep a library around if you're long longer using it or have never used even it.

    You may also consider organizing them in some way, such as alphabetically.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise it'll just get longer if you add more. Also, you have listed using std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • There's no need to provide your own swap() function; just use std::swap:

      std::swap(lit, rit);
    
  • This is not a very common nor maintainable style in my_qsort():

    if( lit > rit ) { break; }
    else { lit++; }
    

    Put each bracketed statement on its own line, in case you ever need to add to them:

      if (lit > rit) {
          break;
      }
      else {
          lit++;
      }
    
  • Why is there a "pause" in my_qsort()?

    char c;
    cin >> c;
    

    Just let the function works without such interruptions. But if you must keep it anyway, then at least make the user aware that it's a pause, and tell them to input a character in order to continue.

  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

  • The last three libraries should be removed as they're not being used. There's no need to keep a library around if you're long longer using it or have never used even it.

    You may also consider organizing them in some way, such as alphabetically.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise it'll just get longer if you add more. Also, you have listed using std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

  • The last three libraries should be removed as they're not being used. There's no need to keep a library around if you're long longer using it or have never used even it.

    You may also consider organizing them in some way, such as alphabetically.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise it'll just get longer if you add more. Also, you have listed using std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • There's no need to provide your own swap() function; just use std::swap:

      std::swap(lit, rit);
    
  • This is not a very common nor maintainable style in my_qsort():

    if( lit > rit ) { break; }
    else { lit++; }
    

    Put each bracketed statement on its own line, in case you ever need to add to them:

      if (lit > rit) {
          break;
      }
      else {
          lit++;
      }
    
  • Why is there a "pause" in my_qsort()?

    char c;
    cin >> c;
    

    Just let the function works without such interruptions. But if you must keep it anyway, then at least make the user aware that it's a pause, and tell them to input a character in order to continue.

  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

added 197 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238
  • Since you'reThe last three libraries should be removed as they're not being used. There's no need to keep a library around if you're long longer using assertionsit or have never used even it.

    You may also consider organizing them in some way, just remove <assert.h>such as alphabetically.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise you'llit'll just end up growing this listget longer if you add more. Also, you have listed using std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

  • Since you're not using assertions, just remove <assert.h>.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise you'll just end up growing this list. Also, you have listed std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

  • The last three libraries should be removed as they're not being used. There's no need to keep a library around if you're long longer using it or have never used even it.

    You may also consider organizing them in some way, such as alphabetically.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise it'll just get longer if you add more. Also, you have listed using std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.

Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238

  • Since you're not using assertions, just remove <assert.h>.

  • You don't really need these:

    using std::vector;
    using std::cin;
    using std::cout;
    using std::endl;
    using std::vector;
    using std::ostream_iterator;
    using std::istream_iterator;
    

    Just use std:: where needed, otherwise you'll just end up growing this list. Also, you have listed std::vector twice, but it should just be there once.

  • print() isn't modifying any data members (it just prints something):

    print( std::vector<float> &data )
    

    so data should be passed by const& to avoid an unnecessary copy:

      print(std::vector<float> const& data)
    
  • Using parentheses for a simple return is not really needed:

    return ( 0 );
    

    Just keep it simple and omit them:

      return 0;
    

    Moreover, you don't really need this. Reaching the end of main() already implies success, so the compiler will insert the return itself.