7

I just could smash my head against the wall. I do not understand, why my following quicksort algorithm is not working. It is written in Pascal:

program quicksort;

Type iArray = array [0..8] of integer;
var intArray, newArray : iArray;

Function getIntArray(localIntArray : iArray) : iArray;
begin
  localIntArray[0] := 3;
  localIntArray[1] := 1;
  localIntArray[2] := 8;
  localIntArray[3] := 4;
  localIntArray[4] := 9;
  localIntArray[5] := 0;
  localIntArray[6] := 8;
  localIntArray[7] := 2;
  localIntArray[8] := 5;
  getIntArray := localIntArray;
end;

Procedure writeArray(localIntArray : iArray);
var i:integer;
begin
  for i:=low(localIntArray) to high(localIntArray) do begin
      write(localIntArray[i]);
  end;
  writeln('');
end;

Procedure quicksort(links : integer ; rechts:integer; localIntArray : iArray); 
// links:      Index des 1. Elements, rechts: Index des letzten Elements
var l, r, pivot, pivotValue, temp : Integer;


    Function swap (larray : iArray; links :integer; rechts:integer) : iArray;                                 
    // Ich tausche in einem IntegerArray die Positionen links und rechts
    var temp : integer;
    begin
        temp                    := larray[links];
        larray[links]           := larray[rechts];
        larray[rechts]          := temp;
        swap                    := larray;
    end;

begin
  if (rechts>links) then begin
    l:= links;
    r:= rechts;
    pivot := localIntArray[(rechts+links) div 2];

    while (l<r) do begin
       while localIntArray[l] < pivot do l:=l+1;
       while localIntArray[r] > pivot do r:=r-1;
       if (l<=r) then begin
         localIntArray := swap(localIntArray,l,r);
         l := l+1;
         r := r-1;
       end;
    end;

    if (links < r) then quicksort(links, r, localIntArray);
    if (rechts > l) then quicksort(l, rechts, localIntArray);

    writeln('s Array: ');
    writeArray(localIntArray);
  end;
end;

var i : integer;
begin

  intArray := getIntArray(intArray);
  writeln('Unsortiertes Array: ');
  writeArray(intArray);
  quicksort(low(intArray), high(intArray), intArray);

end.

When I input the array: 3,1,8,4,9,0,8,2,5 I receive the following calculations:

0,1,2,3,5,4,8,8,9 -> 0,1,2,3,5,4,8,8,9 -> 3,1,2,0,4,5,8,8,9 -> 3,1,2,0,4,5,8,8,9 -> 3,1,2,0,4,5,8,8,9 -> 3,1,2,0,5,4,8,8,9 -> 3,1,8,4,5,0,8,2,9

What is happening here?

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
gutenmorgenuhu
  • 2,312
  • 1
  • 19
  • 33
  • 2
    Why do you not have the array as a `var` parameter to both `quicksort` and `swap`? – 500 - Internal Server Error Jun 20 '14 at 21:18
  • 1
    I want to give the question +1 because it is almost a complete code sample, but we've no idea what `iArray` is. Anyway @500-InternalServerError is correct that you should not be copying the array all over the place, as you do. – David Heffernan Jun 20 '14 at 21:21
  • See also this article, [Implementing QuickSort Sorting Algorithm in Delphi](http://delphi.about.com/od/objectpascalide/a/quicksort.htm). – LU RD Jun 20 '14 at 21:21
  • @David: iArray is obviously an array of integers. – Mason Wheeler Jun 20 '14 at 21:23
  • @MasonWheeler Could be a static array for all you or I know. Could be `TList`. Unlikely I know, but it would change the question. If anybody who worked for me came to me asking for help on this problem, with just this code, they would be asked to come back with a complete problem statement. Please don't encourage posters to post incomplete code. The code in the question is about 5 minutes work away from a complete program. – David Heffernan Jun 20 '14 at 21:25
  • @David: A static array is still an array. The point is that this is complete enough to understand what's going on, and it's admirably well-written considering that the OP's native language is apparently German and not English. – Mason Wheeler Jun 20 '14 at 21:28
  • 2
    @MasonWheeler So the asker can improve the question to include an SSCCE, and we can write good answers with the benefit of a complete set of facts. Nobody has to guess. And such a question would be well worthy of many upvotes. I don't understand why you encourage people to post incomplete code. – David Heffernan Jun 20 '14 at 21:33
  • Thank you for your quick responses. I included the whole code. I just wrote another quicksort algorithm up from scratch and I cannot get it to work either. So the main problem is not any inconvenient/ugly code but a basic understanding issue. Please enlighten me. I feel the urge to strangle myself @LU RD: I do know that article (and many more) tbh I cannot find any differences in my code, – gutenmorgenuhu Jun 20 '14 at 21:40
  • @gutenmorgenuhu Thank you very much for your edit. That is very helpful. Now you get my +1. – David Heffernan Jun 20 '14 at 21:58
  • @gutenmorgenuhu Seems like you are learning to program. So even though you are german, try to code and comment everything in english. It's worth the effort and you will get used to it easily. – Erik Virtel Jul 03 '14 at 07:58

1 Answers1

8

Your program fails because you operate on copies of the array, rather than operating in-place. So consider the declaration of quicksort:

Procedure quicksort(links, rechts: integer; localIntArray: iArray);

The array is passed by value. You pass the array into the procedure, but any changes made to the array are never seen by the caller. Instead you need to operate in place by passing a reference to the array. That is a var parameter:

Procedure quicksort(links, rechts: integer; var localIntArray: iArray);

And you need to do likewise with the swap procedure which should be declared like this:

Procedure swap(var larray: iArray; links, rechts: integer);

Here is a complete program that sorts correctly:

program quicksort24335585;

Type
  iArray = array [0 .. 8] of integer;

var
  intArray: iArray;

Function getIntArray(localIntArray: iArray): iArray;
begin
  localIntArray[0] := 3;
  localIntArray[1] := 1;
  localIntArray[2] := 8;
  localIntArray[3] := 4;
  localIntArray[4] := 7;
  localIntArray[5] := 0;
  localIntArray[6] := 8;
  localIntArray[7] := 2;
  localIntArray[8] := 5;
  getIntArray := localIntArray;
end;

Procedure writeArray(localIntArray: iArray);
var
  i: integer;
begin
  for i := low(localIntArray) to high(localIntArray) do
  begin
    write(localIntArray[i], ' ');
  end;
  writeln('');
end;

Procedure quicksort(links: integer; rechts: integer; var localIntArray: iArray);
// links:      Index des 1. Elements, rechts: Index des letzten Elements
var
  l, r, pivot: integer;

  procedure swap(var larray: iArray; links: integer; rechts: integer);
  // Ich tausche in einem IntegerArray die Positionen links und rechts
  var
    temp: integer;
  begin
    temp := larray[links];
    larray[links] := larray[rechts];
    larray[rechts] := temp;
  end;

begin
  if (rechts > links) then
  begin
    l := links;
    r := rechts;
    pivot := localIntArray[(rechts + links) div 2];

    while (l < r) do
    begin
      while localIntArray[l] < pivot do
        l := l + 1;
      while localIntArray[r] > pivot do
        r := r - 1;
      if (l <= r) then
      begin
        swap(localIntArray, l, r);
        l := l + 1;
        r := r - 1;
      end;
    end;

    if (links < r) then
      quicksort(links, r, localIntArray);
    if (rechts > l) then
      quicksort(l, rechts, localIntArray);
  end;
end;

begin
  intArray := getIntArray(intArray);
  writeln('Unsortiertes Array: ');
  writeArray(intArray);

  quicksort(low(intArray), high(intArray), intArray);

  writeln('s Array: ');
  writeArray(intArray);

  Readln;
end.

Output

Unsortiertes Array:
3 1 8 4 7 0 8 2 5
s Array:
0 1 2 3 4 5 7 8 8

I'm quite sure the program could be polished and improved. Doing so will be part of your learning curve!

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 1
    Oh my God. That is it! Now the chapter "Reference vs. Value" from this week makes more sense =) Thank you so much. I will recode that tomorrow with pass-by-reference to conserve that knowledge – gutenmorgenuhu Jun 20 '14 at 22:04
  • I slept over it and wanted to recode it from scratch, implementing your tip with the call-by-reference var. And again: I cannot find any failure I made again. I compare the code to your program one by one and I cannot find the problem. I added the new code to the original question which produces an Exception (External:SIGSEV at address 11602) during compilation/run – gutenmorgenuhu Jun 21 '14 at 09:59
  • 1
    I cannot comment on that new code. I cannot see it. There's no place for it here. Would have to be a new question. – David Heffernan Jun 21 '14 at 10:00
  • I added it to the original question – gutenmorgenuhu Jun 21 '14 at 10:02
  • 2
    I rolled it back. This question appears to be done. I answered what you asked. You accepted. You can ask a new question. – David Heffernan Jun 21 '14 at 10:03
  • Okay I did so at [quicksort drama#2](http://stackoverflow.com/questions/24340509/quicksort-drama-2) – gutenmorgenuhu Jun 21 '14 at 10:06