DELPHI class with dynamic array has problem with SetLength()

Question

I need to create a class containing an array of record objects but trying to use SetLength raise an Access Violation errror.

Consider the following example of a tree object with fruits.

type
TFruit = record
  color: string;
  weight: double;
end;

type
  TObjectTree = class

  Public
  Fruits: array of TFruit;

  constructor Create;
  procedure AddFruit;
end;

In the implementation, when trying to resize the array of Fruit objects or initializing to nil generates problems.

constructor TObjectTree.Create;
begin
    inherited Create;
    Fruits:=nil;              //Raises an error
end;


procedure TObjectTree.AddFruit(FruitColor: string; FruitWeight: integer);
begin
    SetLength(Fruits, Length(Fruits)+1);  //Raises an error (when I comment Fruits:=nil; in the constructor)

    Fruits[Length(Fruits)].color:=FruitColor;      
    Fruits[Length(Fruits)].weight:=FruitWeight;
end;

How can I use dynamic arrays in a class?

iamjoosy · Accepted Answer · 2011-07-26 16:24:33Z

13

Replace

Fruits[Length(Fruits)].color:=FruitColor;      
Fruits[Length(Fruits)].weight:=FruitWeight;

with

Fruits[High(Fruits)].color:=FruitColor;
Fruits[High(Fruits)].weight:=FruitWeight;

then it works.

answered Jul 26, 2011 at 16:24

iamjoosy

3,3591 gold badge24 silver badges31 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Lars Truijens Over a year ago

The explanation is that dynamic arrays are 0 based and run to Length - 1.

polarized Over a year ago

Right, this is also a bug in the example.

Rob Kennedy · Accepted Answer · 2011-07-26 17:11:08Z

9

Something tells me you've neglected to create an instance of TObjectTree. You've declared a TObjectTree variable, but you've either not called TObjectTree.Create, or you've called it directly on the variable you declared instead of assigning a new value to that variable:

var
  Tree: TObjectTree;
begin
  // This is wrong.
  Tree.Create;

  // This is right.
  Tree := TObjectTree.Create;

Without properly instantiating TObjectTree, there is no valid memory to back the Fruits field you attempt to use, so assigning a value to it gives an error.

answered Jul 26, 2011 at 17:11

Rob Kennedy

164k23 gold badges288 silver badges481 bronze badges

3 Comments

David Heffernan Over a year ago

+1 this more likely to explain goings on than my memory corruption explanation

polarized Over a year ago

Absolutely right. This was causing the memory problems. Now it is possible to instantiate a TreeObject and set the Fruits array to nil (to evantually clear the added fruits later on by calling a class method to be implementd).

David Heffernan Over a year ago

it's pointless to set Fruits to nil in the constructor. It is already nil.

David Heffernan · Accepted Answer · 2011-07-26 18:53:20Z

6

As an addition to the answers of iamjoosy and Rob Kennedy, I would code this like so:

procedure TObjectTree.AddFruit(FruitColor: string; FruitWeight: integer);
var
  NewCount: Integer;
begin
  NewCount := Length(Fruits)+1;
  SetLength(Fruits, NewCount);
  Fruits[NewCount-1].color := FruitColor;      
  Fruits[NewCount-1].weight := FruitWeight;
end;

It is clearer, in my view, to call Length() just once.

You do not need to assign Fruits := nil in the constructor since that happens automatically. All fields are zero-initialised when an object is instantiated. That said, Fruits := nil should not raise an error. If it does it is probably a result of a memory corruption due to the out-of-bounds array accessing.

A further point to make is that enabling range checking would have resulted in an informative error that would have explained the problem. This is much more helpful than relying on access violations. I can't recommend range checking highly enough.

Finally, the SetLength(..., Length(...)+1) pattern typically leads to very inefficient memory usage and can lead to performance problems for large lists. If you have Delphi 2009+, I would recommend using TList<TFruit> instead.

edited Jul 26, 2011 at 18:53

answered Jul 26, 2011 at 16:35

David Heffernan

616k46 gold badges1.1k silver badges1.5k bronze badges

2 Comments

Arnaud Bouchez Over a year ago

I usually use a function for such code, returning an integer which is the index of the newly created item, then use result := length(Fruits); SetLength(Fruits,result+1); Fruits[result].color := FruitColor; It does make more sense to me than those +1/-1 around, and it is sometimes handy to retrieve the newly created index for the caller, with no speed penalty. Just my 2 cents.

David Heffernan Over a year ago

@Arnaud Agreed, although I'd just use TList<TFruit> which of course does exactly that.

Collectives™ on Stack Overflow

DELPHI class with dynamic array has problem with SetLength()

3 Answers 3

2 Comments

3 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related