Skip to main content
added 103 characters in body
Source Link
vnp
  • 58.7k
  • 4
  • 55
  • 144

First of all, the code looks correct. Kudos.

Few notes though.

  • No naked loops. Every loop implements an important algorithm, and therefore deserves a name. In your case, the loop

      for (int* i = p+1; i <= tmpStop; ++i)
    

    partitions the range. Lift it into a partition function. Not only it will make the code more readable, but also more testable. Now you can unit test partition. And more maintainable too: you may change the partitioning strategy and not touch the core of sorting.

  • Most range algorithms are more naturally expressed in terms of a semi-open range (that is, the end does not belong to the range, but is one-beyond). With such contract you don't need to -1 anywhere, and you may use < instead of <=.

  • I am not sure that std::vector is the best choice to represent the stack. STL conveniently offers a stack container.

  • flip is likely inferior to std::swap.

  • Stop using namespace std. It is a bad habit.

First of all, the code looks correct. Kudos.

Few notes though.

  • No naked loops. Every loop implements an important algorithm, and therefore deserves a name. In your case, the loop

      for (int* i = p+1; i <= tmpStop; ++i)
    

    partitions the range. Lift it into a partition function. Not only it will make the code more readable, but also more testable. Now you can unit test partition.

  • Most range algorithms are more naturally expressed in terms of a semi-open range (that is, the end does not belong to the range, but is one-beyond). With such contract you don't need to -1 anywhere, and you may use < instead of <=.

  • I am not sure that std::vector is the best choice to represent the stack. STL conveniently offers a stack container.

  • flip is likely inferior to std::swap.

  • Stop using namespace std. It is a bad habit.

First of all, the code looks correct. Kudos.

Few notes though.

  • No naked loops. Every loop implements an important algorithm, and therefore deserves a name. In your case, the loop

      for (int* i = p+1; i <= tmpStop; ++i)
    

    partitions the range. Lift it into a partition function. Not only it will make the code more readable, but also more testable. Now you can unit test partition. And more maintainable too: you may change the partitioning strategy and not touch the core of sorting.

  • Most range algorithms are more naturally expressed in terms of a semi-open range (that is, the end does not belong to the range, but is one-beyond). With such contract you don't need to -1 anywhere, and you may use < instead of <=.

  • I am not sure that std::vector is the best choice to represent the stack. STL conveniently offers a stack container.

  • flip is likely inferior to std::swap.

  • Stop using namespace std. It is a bad habit.

Source Link
vnp
  • 58.7k
  • 4
  • 55
  • 144

First of all, the code looks correct. Kudos.

Few notes though.

  • No naked loops. Every loop implements an important algorithm, and therefore deserves a name. In your case, the loop

      for (int* i = p+1; i <= tmpStop; ++i)
    

    partitions the range. Lift it into a partition function. Not only it will make the code more readable, but also more testable. Now you can unit test partition.

  • Most range algorithms are more naturally expressed in terms of a semi-open range (that is, the end does not belong to the range, but is one-beyond). With such contract you don't need to -1 anywhere, and you may use < instead of <=.

  • I am not sure that std::vector is the best choice to represent the stack. STL conveniently offers a stack container.

  • flip is likely inferior to std::swap.

  • Stop using namespace std. It is a bad habit.