Skip to main content
Code Review

Return to Revisions

1 of 2
greybeard
  • 7.4k
  • 3
  • 21
  • 55

the goal is to determine whether [pivot selection as average of extrema] is comparable to median-of-three
which probably is to say How does pivot selection as presented here compare to MoT?
If that is goal of this post rather than goal of the code presented, CR is the wrong forum for that.

(These preliminary remarks done with, I'll more or less follow the source top to bottom.)

Personal dislikes:
- code where I don't know at first glance
a) what the idea was creating it
b) what I can ethically do with it.
- waste of screen space (viz. circumventing SE's bulleted list here)

I suggest leaving license information out of doc comments, using blank lines to separate things not (visually) separated otherwise, only, and using markup sparingly and with as little intrusion as feasible (e.g., </p> at end of last line of paragraph).

I miss a description about
- what you found concerning
- how to compare quicksort pivot selection schemes
- how the scheme you present relates to prior work
(which would help presenting a similar question at CS.)
- the raison d'être of this code
(And yes, I'm talking source code. External documentation does get separated. I'd be fine with a hyperlink given any hope of years of accessibility. I don't want to exercise my skills in finding something on the net.)

  • class QSortSAVP (what does the 2nd S stand for?) consider separating recursion handling, pivot selection (done), partitioning, and "driver" (main() looks the type - I'd prefer a separate/nested class).
    Implement an interface Sorter<T>.
  • As such, first step is... is this comment up to date? (Where would arithmetic overflow become a problem? See determinePivot()'s doc comment, too. You should be able to get rid of the (current) special case for firstIteration.)
  • [] sort([], boolean createCopy) looks excessive for a non-production strength implementation. I'd prefer mentioning the case for a copy in sort([] arr)'s doc comment (especially if that returned its argument) or [] sortCopy([]).
    Why open code [Arrays.copyOf([], int newLength)](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Arrays.html#copyOf(T%5B%5D, int))?
    sort([] arr, pivotVal, int lowIndex, int highIndex, ...)
    - Intrusive tracing impedes readability. If need be, use an appropriate package.
    - I don't quite get the // all entries in given range are less than or equal to pivot (which they conceivably are) - the return; is in order because determinePivot()never rounds up (which I don't find documented anywhere) and all entries are equal
    - not passing (in some convenient way) information about pivot selection between invocations of this method and int determinePivot([] arr, int lowIndex, int highIndex) throws away information: the overall low from lowIndex to highIndex will stay the low from lowIndex to tempLowIndex, analogously for high.
    - If at all, just handle (MAX_)RECURSION_DEPTH where you handle RECURSION_COUNT
    - determine the pivot at top of this method instead of before each recursive call
  • void handlePrint(object)& ...Line() see suggestion about logging/tracing package
greybeard
  • 7.4k
  • 3
  • 21
  • 55
default

AltStyle によって変換されたページ (->オリジナル) /