Advice on clunky code please

[LEICJDN1]

Hi,

Application is complete and works fine. However, nagging suspicion my code is very inefficient and 'clunky' if you know what i mean.

The body of my app handles several different images, each of which is displayed in turn. For each image, there is a unique name, several unique string / integer / real / array variables. At the moment I have a counter which informs the code which image is being analysed. Then I have a huge long list of:

If imagecounter:=1 then...

Namelabel.caption:='Image1'
Image1X:=
Image1Y:=

... main procedure...

If imagecounter:=2 then...

Namelabel.caption:='Image2'
Image2.X:=
Image3.Y:=

... main procedure...

etc.

So, my 'main procedure' occurs many times in the code, for each possible count of imagecounter.

I know i can set my 'main procedure' up as a procedure on it's own, and am happy sending variable to and from it. However, how can you have 'variable dependent variables'??

For instance, how can my seperate main procedure read in the image counter, and then modify the appropriate variables for that image without directly specifying it in a load of if.. then statements?
Can you have a 'look up' table of variable names? Can you have a variable which 'points' to the appropriate variable name?

I am sure this is just a conceptual thing in Delphi I am not fmailiar with, and i hope I have explained myself clearly.

Altough my app works great, and is only 2Mb in size, i would still like to tidy it up and optimise the code if I can.

Thanks

 

[LucieLastic]

If it were me I would have created a BaseImage type object. BaseImage would have properties for x,y and the image it contains and any image management routines.

I would have something like a TList containing all the created BaseImage objects. Then loop through the list and call the appropriate routine, eg

for ii := 0 to ImageList.count-1
PBaseImage(ImageList[ii]).RoutineName;

I have a similar project and I have a controlling unit which holds the list of objects(in this case Tforms) and finds/manages the appropriate form depending what the user wants to do.

Hope this gives you some ideas.

lou

 

[colttaylor]

Is this what you are looking for...?

  const max_img = 3;
  type  img_rec = record
                  img      : timage;
                  filename : string[80];
                  x,y      : integer;
                  end;
        img_arr = array[1..max_img] of img_rec;

  procedure handle_image(ndx : integer);
    begin
    if ndx in [1..max_img]
    then begin
         with img_arr[ndx]
         do begin
          { ...do your main procedure 
               here using img, filename, 
               x and y fields....... }
            end;
         end;
    end;
  
  procedure load_image(ndx          : integer;
                       new_filename : string;
                       new_x,new_y  : integer);
    begin
    if ndx in [1..max_img]
    then begin
         with img_arr[ndx]
         do begin
            img := timage.create;
            filename := new_filename;
            if fileexists(filename)
            then img.loadfromfile(filename);
            x := new_x;
            y := new_y;
            end;
         end;      
    end;

  begin
  load_image(1,'image1.bmp',10,10);
  load_image(2,'image2.bmp',20,05);
  load_image(3,'image3.bmp',01,01);
{ start the rest of your program logic here }  
  end.

If I missed what you are looking for, let me know.
Peace,
Colt.

If it's stupid but it works, it isn't stupid

 

[LEICJDN1]

Thanks,

Will work through the above, but at first glance looks spot on.