What I should change in this Delphi code, to get the right result?

Question

In this Delphi code, should be store the minmum value into newA and delete it from A , this repeated until A reached to the size 0

type TDoubleDynArray  = array of Double;

          ..............

            ..........

//Get minimum value:

function MinValue(var minPos : Integer; Data: TDoubleDynArray): Double;
var
  I: Integer;
begin
  ShowMessageUser(IntToStr( Length(data)));
  Result := Data[Low(Data)];
  for I := Low(Data) + 1 to High(Data) do
    if Result > Data[I] then
      minPos := i;

  Result := Data[minPos];
end;

//-------------------------Main function ------------------

function TForm1.Func(X: TDoubleDynArray): Double;
var
A, newA : TDoubleDynArray;
i, minPos : Integer;
begin

   SetLength(A, 55);
   SetLength(newA, 55);
   A := Copy(X,0, 55);


      for i := 0 to 54 do
      begin
        newA[i] := MinValue(MinPos,A);
        Delete(A, MinPos, 1);
      end;

           ..............

            ..........


 end;

When i run the code it stop on size 3 (length of A = 3):

The Result of function MinValue() //ShowMessageUser(IntToStr( Length(data)));

55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32 31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 3 3

What I should change to get the right result (length(A) := 0)???

Just set `minPos` value in `MinValue` function to `Low(Data)` at first line :) P.S. have you tried to debug your code? — kami, Mar 02 '16 at 05:49
MinValue has multiple errors. However you should throw away all of the code and just sort the array with a decent sort algo. Your selection sort is O(n^2). Delphi has built in functions to sort. — David Heffernan, Mar 02 '16 at 06:34
[`TArray.Sort`](http://docwiki.embarcadero.com/Libraries/en/System.Generics.Collections.TArray.Sort) — David Heffernan, Mar 02 '16 at 07:14
@David, I'd have assumed that implementing selection sort was the *purpose* of this student's exercise (perhaps as a precursor to implementing a better algorithm and observing the differences). — Rob Kennedy, Mar 02 '16 at 15:30

David Heffernan · Accepted Answer · 2016-03-02T07:31:21.273

4

There are numerous problems with your code. Most obviously, MinValue is badly broken. It fails to set minPos when the first value in the array is the lowest. That explains why your code fails.

However, you should probably remove all of the code and use the built in sorting functions. Your hand rolled selection sort doesn't work (yet) and even when fixed will still be O(n²).

Use TArray.Sort to perform the sort.

var
  SortedX: TArray<Double>;
....
SortedX := Copy(X, 0, 55);
TArray.Sort<Double>(SortedX);

This uses Quicksort which has a time complexity of O(n log n) on average.

Even if your version of Delphi predates TArray.Sort, conceptually you should be using a well proven sort algorithm, encapsulated cleanly. Don't roll your own ad-hoc sorting code inline like this.

As a minor aside, it's pointless to create a new array, and then throw it away as you do in this code:

SetLength(A, 55);
A := Copy(X, 0, 55);

The call to SetLength is pointless and wasteful because the newly created array is thrown away when replaced by the new array returned by Copy.

edited Mar 02 '16 at 07:31

answered Mar 02 '16 at 07:21

David Heffernan

601,492
42
1,072
1,490

how can I choose the type of sorting (descending or ascending) – shdotcom Mar 02 '16 at 07:31
1

Pass a custom comparer to `Sort`, see the doc link – David Heffernan Mar 02 '16 at 07:32
what if I want to use this type var SortedX: TDoubleDynArray – shdotcom Mar 02 '16 at 07:39
I have got this error Undeclared identifier: 'TArray' – shdotcom Mar 02 '16 at 07:45
The easiest way is to call `TComparer.Construct` – David Heffernan Mar 02 '16 at 07:46
Do bear in mind that I don't know what delphi version you use. If it's a modern version you should use the generic dyn array `TArray` for type compatibility reasons. If you don't have generics then you should use a general sort code suited to whateve delphi version you have. – David Heffernan Mar 02 '16 at 07:48
Yes, you should do that everywhere so that your code is more flexible and composable – David Heffernan Mar 02 '16 at 07:52
@David. Off topic: On my Delphi (XE3) Delete()'s help only notes strings and not dynamic arrays. Online help does note dynamic arrays. When was it expanded to dynamic arrays? – Blurry Sterk Mar 02 '16 at 07:54
@ David Heffernan can I do like this : uses ............. type TDoubleDynArray = TArray; – shdotcom Mar 02 '16 at 08:00
Why would you want to do that? – David Heffernan Mar 02 '16 at 08:01
@Blurry XE8 I think. If I'd been cleverer I could have used that to infer asker's version. – David Heffernan Mar 02 '16 at 08:02
You don't need to use the generic array if you don't want to. But there are advantages of doing so: http://stackoverflow.com/a/14383278/505088 – David Heffernan Mar 02 '16 at 08:06
@David Heffernan I have changed all to TArray; but why I get this error : Undeclared identifier: 'TArray' – shdotcom Mar 02 '16 at 08:06
1

You'll encounter that with `TArray.Sort` unless you include the unit which declares `TArray`. Which is documented in the link I provided. It's `System.Generics.Collections`. – David Heffernan Mar 02 '16 at 08:18
@David Heffernan what about multidimensional Array how can I use "TArray" in this case? – shdotcom Mar 02 '16 at 08:25
1

`TArray>` – David Heffernan Mar 02 '16 at 08:32
@David Heffernan Thank you very much for your help. – shdotcom Mar 02 '16 at 08:39

Blurry Sterk · Answer 2 · 2016-03-02T07:24:28.020

2

The problem seems to be in the MinValue function..

From what I can see you want to get the lowest value in the array. Currently you are only getting the last element in the array of lower value than the first element in the array irrespective of whether it is the lowest value in the total array. That happens because you keep on comparing to the Result which was set to the first element of the array and it never gets updated.

The code below gives back the lowest value in the array.

function MinValue(var minPos : Integer; Data: TDoubleDynArray): Double;
var
  I: Integer;
begin
  minPos := 0;

  for I := 1 to High(Data) do
    if Data[I] < Data[minPos] then
      minPos := I;

  Result := Data[minPos];
end;

edited Mar 02 '16 at 07:24

answered Mar 02 '16 at 05:53

Blurry Sterk

1,595
2
9
18

No, what I want is take the lowest value from A and put it in another array, then delete it from A, and keep doing this until the length of A = 0 – shdotcom Mar 02 '16 at 06:25
1

@shdotcom You cannot take the lowest value with wrong MinValue function – MBo Mar 02 '16 at 06:36
This should set minPos to -1 if the array has length 0 I think – David Heffernan Mar 02 '16 at 07:29
@David. Agreed. If he keeps using it in the for loop as he is now then it will not be needed. But it is always a good idea to do the preventative code aspecially if he changes the calling code. – Blurry Sterk Mar 02 '16 at 07:41

What I should change in this Delphi code, to get the right result?

2 Answers2