4

I apologize in advance for a newbie question, but why do I get "Access violation" error with the code below (on the "Create(SelectorForm);" line)? I tried using the main form as the owner, but it didn't make any difference.

var
  SelectorForm: TSelectorForm;
  ArrayOfImages: Array [1..10] of TImage;

implementation

procedure TSelectorForm.FormCreate(Sender: TObject);
var
  Loop: Byte;
begin
  for Loop := 1 to 10 do
  begin
    with ArrayOfImages[Loop] do
    begin
      Create(SelectorForm);
    end;
  end;
end;
Mikhail
  • 1,326
  • 6
  • 22
  • 29

2 Answers2

18

The problem is that you are effectively doing this:

var
  imageVariable: TImage;
begin
  imageVariable.Create (ParentForm);
end;

Which is wrong because "Create" method is being called on the variable which hasn't been assigned yet.

You should do this:

var
  imageVariable: TImage;
begin
  imageVariable := TImage.Create (ParentForm);
  try
    //use the object
  finally
    FreeAndNil (imageVariable);
  end;
end;

Or more specifically in your code:

for Loop := 1 to 10 do
begin
  ArrayOfImages[Loop] := TImage.Create (Self);
end;

Don't forget to free the objects

EDIT: Accepting @andiw's comment and taking back the tip of freeing objects. EDIT2: Accepting @Gerry's comment and using Self as owner.

Hemant
  • 19,486
  • 24
  • 91
  • 127
  • In my understanding the TImages will be freed automatically when SelectorForm is destroyed since it's been passed as their owner - or am I getting something wrong? – Andreas Wieland Jun 24 '09 at 07:15
  • @andiw: You are right. There is no need to free the objects if you assign the form as their owner. – Hemant Jun 24 '09 at 08:15
  • 3
    Wouldn't TImage.Create(Self); be better. It's nearly always a bad idea to use a specific form instance variable within a method of the form's class - it will fail if someone creates an instance with another name. – Gerry Coll Jun 24 '09 at 21:31
0

There are a lot of problems with the above code. (don't use the "With" like that for a start, don't use a Byte for your loop var)

My assumption is that you ultimately want an array of instances of TImage's created with a form as the parent.

so based on that assumtion...you want something like (untested)

var
  ArrayOfImages: Array [0..9] of TImage;  
  i : integer;
begin
  for i := 0 to 9 do
  begin
    ArrayOfImages[i] := TImage.Create(theForm);
  end;

end;

Now note, you will be responsible for cleaning up the array when you are finished using it, you will need to call free on each of the Image instances.

Tim Jarvis
  • 18,465
  • 9
  • 55
  • 92
  • 1
    Using byte for the loop variable certainly doesn't qualify as a problem. – mghie Jun 24 '09 at 06:24
  • 2
    nope, just a style thing. Nothing wrong with limiting the range for the loop, but byte is typically a meaningful size, I would be spending a couple extra secs looking at this and wondering why the byte... – Tim Jarvis Jun 24 '09 at 06:40
  • No, he doesn't need to clean the instances. theForm will do it. And the array is a global. It'll die when the unit is finalized. – Fabricio Araujo Jun 24 '09 at 23:29