1

I'm trying to create a component descending from TImage, with the difference that I can assign a variable number of TPictures in the property list (not assign the TPictures by code) and activate one of them by code to be displayed in the TImage.

It would not be a problem to have a property that sets the overall number of TPictures (length of the dynamic array), if that's necessary to have all the TPictures assignable within the properties.

unit ImageMultiStates;

interface

uses
  Vcl.Graphics, Vcl.StdCtrls, System.SysUtils, System.Classes, Vcl.Controls, Vcl.ExtCtrls, Forms;

type
  TPictures = Array of TPicture;
  TImageMultiStates = class(TImage)
  private
      FPictures: TPictures;
      procedure SetPicture(Which: Integer; APicture: TPicture);
  public
      constructor Create(AOwner: TComponent); override;
      destructor Destroy; override;
      procedure Activate(Which: Integer);
  published
    property Images: TPictures read FPictures write FPictures; default;
  end;

procedure Register;

implementation

constructor TImageMultiStates.Create(AOwner: TComponent);
begin
  inherited Create(AOwner);
  for TPicture in FPictures do
    TPicture := TPicture.Create;
end;

destructor TImageMultiStates.Destroy;
var
  APicture: TPicture;
begin
  for APicture in FPictures do
    APicture.Free;
  inherited Destroy;
end;

procedure TImageMultiStates.Activate(Which: Integer);
begin
  Picture.Assign(FPictures[Which]);
end;

procedure TImageMultiStates.SetPicture(Which: Integer; APicture: TPicture);
begin // i would also like to use SetPicture instead of "write FPictures"
  FPictures[Which].Assign(APicture);
  if Which=0 then // because: First Picture will be displayed in the VCL editor
    Picture.Assign(FPictures[Which]);
end;

procedure Register;
begin
  RegisterComponents('Standard', [TImageMultiStates]);
end;

end.

I turned this code around in many ways, but i just can't get anything to work really.

I do have this exact same idea already working in my component 'Image2States'. Then I needed 'Image4States' and so on, until I decided that I absolutely need this with a variable amount of TPictures...

hzzmj
  • 121
  • 11
  • 1
    You need to maintain a collection of pictures. Hoping that you can create the picture instances in the constructor is a mistake. – David Heffernan Jul 21 '16 at 06:17
  • @David Heffernan: Oh yes of course. Silly me. But other than that, what I'm trying here could work? Should i go for the Setting of the TPicture amount within the properties, or will a collection of TPictures be displayed correctly by an appropriate PropertyEditor anyways? – hzzmj Jul 21 '16 at 06:24
  • I'd imagine that you want to use a `TCollection` containing your picture objects. Then it should hook up to the property editor readily. – David Heffernan Jul 21 '16 at 06:29
  • In the loop `for TPicture in FPictures do`, TPicture **is not declared anywhere**, hence it is obviously **not a variable**, so it must be the class type. I am a bit surprised that this actually compiles. In another loop, in `Destroy`, you have a proper `APicture` variable of the right type. – Rudy Velthuis Jul 21 '16 at 06:38
  • 1
    Just in case, you do know about `TImageList`, do you? It can hold a dynamic amount of **equally sized** images of any type that TImage supports and has a design time editor. The limitation is that the images need to be the same size, default is 16 x 16 but you can change height and width as you wish before any images are added to the list. – Tom Brunberg Jul 21 '16 at 07:28
  • I tested this line: `var Idiocy: TArray; ... for TNonsense in Idiocy do` and got an `E1019 For loop control variable must be simple local variable`. This makes me think that the code you show here is not the real code. How can we help if you post fake code? – Rudy Velthuis Jul 21 '16 at 07:53
  • @Rudy Velthuis: I said: "I Can't get it to work." It doesn't work. I don't know how to make it work. That's the question. I had a ton of modified Versions of this, travelling between them as we spoke. This is the one i ended up with, just to communicate the basics of what I'm trying to achieve. It doesn't compile. – hzzmj Jul 21 '16 at 08:13
  • Apart from the fact that what you try to do doesn't make sense, if you want to loop, you do it as in the destructor. Do what others say: use a collection. Since this is more or less a homebrewn version of a `TImageList`, you could use a `TObjectList` or similar. If you don't have generics, then use a `TObjectList` from *contnrs.pas* and cast (using `as`) where that is necessary. – Rudy Velthuis Jul 21 '16 at 08:17
  • And @hzzmj, "I can't get it to work" is pretty vague. Show us the exact text of the error message(s) you got. Usually such error texts explain what is wrong, so read them. Do not turn of any warnings or hints. Listen to your compiler. – Rudy Velthuis Jul 21 '16 at 08:20
  • @Rudy Velthuis: Given my general description of the goal in the first lines, i was questioning my own approach in the first place. This is less about fixing the one exact compiler error thrown at this moment, but more about with which structure my goal can be achieved. After all, David Heffernan's Collection hint will probably be the answer, but it's a comment. So i accepted what pointed to the very thinking mistake regarding compilation errors, because it also helped me to understand. – hzzmj Jul 21 '16 at 08:35
  • 1
    But consider using a TImageList, if you have pictures of the same size, e.g. indicating different states of a control. A TImageList can even mask pictures, so they don't even have to appear rectangular, if you don't want them to. – Rudy Velthuis Jul 21 '16 at 09:20
  • However, a `TImageList` only support images of the same dimensions, and it only supports bmp and ico images. In recent Delphi versions, png images can be assigned to a `TImageList` at design-time, but they are simply converted to bmp images (the `TImageList` must be set to 32bit for this to work correctly). – Remy Lebeau Aug 05 '16 at 01:58

2 Answers2

1

I would answer this by asking: What are you expecting this to do:

for TPicture in FPictures do
  TPicture := TPicture.Create;

Firstly, as written, this simply does not compile. The TPicture loop variable is not declared (and being the same as the name of a type would cause subsequent compilation errors even if it were declared).

Even assuming that the loop were valid, compilable code, when the constructor executes FPictures is an empty array so this loop will execute 0 (zero) times. You do not show any code which tells the component how many pictures it should support so it always supports 0 (zero).

The problems don't end there.

If you did declare an appropriate variable for use as the loop variable, the loop code will still not compile since it involves an assignment to the loop variable which is not permitted.

This is true even in simple for-loops as well as iterator based loops such as the one you were attempting to use. In the case of simple loops, this is to enable the compiler to produce optimal code. In the case of iterator loops it also protects you from mistakes that might otherwise be easy to make.

Consider that in this case on each iteration of the loop, you would create a new TPicture instance and assign it to the loop variable but this does not assign it to the item in the array from which the loop variable is initialised.

Perhaps the easiest way to explain this is to "unroll the loop" (Unrolling a loop is re-writing the code explicitly for each iteration, as if there were no loop at all).

So consider if the loop contained 2 items, and let's also change the name of the loop variable to make things both valid as well as a bit clearer. We'll use simply pic. In other words we'll unroll a 2 iteration version of this loop:

for pic in FPictures do  // fPictures contains 2 items
  pic := TPicture.Create;

Remember, this does not compile and the reason why this helps prevent us making mistakes becomes apparent when we unroll the loop that it would create, if it were possible:

// Iteration #1
pic := fPictures[0];
pic := TPicture.Create;

// Iteration #2
pic := fPictures[1];
pic := TPicture.Create;

Hopefully you see the problem. You overwrite the value of the loop variable on each iteration, but this does not modify the array items themselves. Worse than that, as a result, each iteration of the loop is leaking a TPicture.

Hopefully this should now help you understand why your code isn't working (actually, can't work) and make the necessary corrections.

  1. You need some mechanism to set the number of pictures supported by your component

  2. You need to correctly initialise the items in the array holding those pictures

Deltics
  • 22,162
  • 2
  • 42
  • 70
  • FWIW, the loop does not compile, because TPicture is not a local variable (it is, in fact, the type). So what is the real code? – Rudy Velthuis Jul 21 '16 at 08:04
  • Even if the loop variable is a variable of type TPicture, e.g. `var N: TPicture; begin for N in FPictures do N := TPicture.Create;` you get a **Let side cannot be assigned to** error, since you cannot assign to the loop variable. Your entire post seems to dissect that loop, but apparently it didn't occur to you that this can't compile. **TPicture was not declared** and thus not a local variable, but the type itself, **and you can't assign to the loop control variable.** – Rudy Velthuis Jul 21 '16 at 08:33
1

You told us that your approach "didn't work" (I assume you meant "didn't compile"): That is because of a few syntactical errors and because you are not using the right tool for the job.

If you only have pictures of the same size, then think about using the already existing, well tested and IDE-supported TImageList and don't try to reinvent the wheel.

Try...

If, however, you must have a list of pictures of different sizes, then use a TObjectList and not an array of TPicture. TObjectLists allow you to add, remove, query, etc. objects and they can automatically free them, if you wish.

If your compiler supports generics, then include System.Generics.Collections and use a TObjectList<TPicture> to manage your pictures. That way, you don't have to cast to TPicture, because the generics lists are type safe.

If it doesn't support them, include the unit Contnrs and use a TObjectList. When reading from that list, you will have to cast using as, i.e. as TPicture, but otherwise you can do similar things.

Finally...

The name of your type makes me think you only need multiple states for a certain control. In that case, I think a TImageList is the best tool for the job (and it already is for other controls with similar needs) and there is no need to make your own one. But if you want to make your own, don't use a dynamic array and don't produce loops like the one with for TPicture in FPictures do. Do yourself a favour and use an object list.

End

Community
  • 1
  • 1
Rudy Velthuis
  • 28,387
  • 5
  • 46
  • 94