Optimal Code problem #1

[Traveler]

This is the first problem of the Optimal Code problem thread. For more information about the Optimal Code event, click here (http://www.pascalgamedevelopment.com/forum/index.php?topic=5867.0).

Problem description:
I have a list of 50 TImage components. They each have an image preloaded in design time. I also have a variable that holds a number between 1 and 50. I want to check the variable for the value and then display the corresponding image from the correct TImage component.

First solution:


procedure textureTile(tile :TTileInfo; tilenr: byte);
var i : byte;
name : string;
begin
case tilenr of
1: Map.Canvas.Draw(tile.x*32,tile.y*32, image1.Picture.Graphic);
2: Map.Canvas.Draw(tile.x*32,tile.y*32, image2.Picture.Graphic);
3: Map.Canvas.Draw(tile.x*32,tile.y*32, image3.Picture.Graphic);
4: Map.Canvas.Draw(tile.x*32,tile.y*32, image4.Picture.Graphic);
5: Map.Canvas.Draw(tile.x*32,tile.y*32, image5.Picture.Graphic);
6: Map.Canvas.Draw(tile.x*32,tile.y*32, image6.Picture.Graphic);
7: Map.Canvas.Draw(tile.x*32,tile.y*32, image7.Picture.Graphic);
8: Map.Canvas.Draw(tile.x*32,tile.y*32, image8.Picture.Graphic);
9: Map.Canvas.Draw(tile.x*32,tile.y*32, image9.Picture.Graphic);
10: Map.Canvas.Draw(tile.x*32,tile.y*32, image10.Picture.Graphic);
11: Map.Canvas.Draw(tile.x*32,tile.y*32, image11.Picture.Graphic);
12: Map.Canvas.Draw(tile.x*32,tile.y*32, image12.Picture.Graphic);
(..)
50: Map.Canvas.Draw(tile.x*32,tile.y*32, image50.Picture.Graphic);
end;
end;

10 lines is doable, 50 is not. There must be a better way, especially because the only thing that changes is the value and the name of the image component.

Alternative solution:

procedure textureTile(tile :TTileInfo; tilenr: byte);
var i : byte;
name : string;
begin
for i := 0 to ComponentCount - 1 do
if Components[i] is TImage then
begin
name := (Components[i] as TImage).Name;
delete(name,1, length('Image'));
if strToInt(name) = tilenr then
begin
map.Canvas.Draw(tile.x*32,tile.y*32, (Components[i] as TImage).Picture.Graphic);
break;
end;
end;
end;

Looks better, but I don't like the loop, especially because this procedure is called a lot, which means that its probably slower than the first solution. Perhaps there is a better way to handle this?

[cragwolf]

I haven't used Delphi (I assume that's what you're using) in years, but surely something like this would work:


procedure textureTile(tile :TTileInfo; tilenr: byte);
begin
Map.Canvas.Draw(tile.x*32,tile.y*32, TImage(FindComponent('image' + IntToStr(tilenr))).Picture.Graphic);
end;

Of course, to be safe you would probably want to check the output of FindComponent.


Traveler: added pascal tags.

[chronozphere]

var
Graphs: array [1..50] of TGraphic;

implementation

//....

//Call this one first to initialize the array
procedure TForm1.BuildImageArray();
var i : byte;
a : integer;
name : string;
begin
for i := 0 to ComponentCount - 1 do
if Components[i] is TImage then
begin
name := (Components[i] as TImage).Name;
delete(name,1, length('Image'));
if TryStrToInt(name, a) then
Graphs[a] := (Components[i] as TImage).Picture.Graphic;
end;
end;

//This is just a simple lookup routine
procedure TForm1.textureTile(tile :TTileInfo; tilenr: byte);
begin
if Assigned(Graphs[tilenr]) then
Map.Canvas.Draw(tile.x*32,tile.y*32, Graphs[tilenr]);
else
raise EInvalidOperation.Create('Tile-Graphic with index: '+Inttostr(tilenr)+' is not assigned.');
end;

;)


Traveler: added pascal tags and corrected a few code errors to make it work.

[Traveler]

Great replies guys! That's exactly what I was hoping for.

@cragwolf: That's the simplest solution, I believe. And it can't get any shorter too.
@chronozphere: I like your solution a lot too. It does the hard work (looking for the correct component) only once. I had never thought of doing something like that.

In my case speed isn't really an issue, so I'm going for cragwolf's solution. Of course if there are any other solutions, I'm interested in hearing them as well.
In the meantime, I already have received a new problem that I'll be posting very soon.

[arthurprs]

chronozphere aproach is "optimal", just an array lookup :yes:

[Andreaz]

I agreee with arthurprs, it could be done "faster", but not in the realm of the problem. (hw acceleration)