Create TTimer in loop

Question

How can I replace my noob code with nice loop ? I have 32 timers on form1 , every timer runs every 1 second and execute bat file, then waiting until bat file finishes his job and timer runs again.

Here is the code from image

procedure TForm1.Timer1Timer(Sender: TObject);
var nr:string;
begin
  nr := '1';
  Timer1.Enabled := False;

  if g_stop=False then
  begin
    if FileExists('test'+nr+'.bat') then
    begin
      ExeAndWait(ExtractFilePath(Application.ExeName) + 'test'+nr+'.bat', SW_SHOWNORMAL);
    end;
    Timer1.Enabled := True;
  end;
end;



procedure TForm1.Timer2Timer(Sender: TObject);
var nr:string;
begin
  nr := '2';
  Timer2.Enabled := False;

  if g_stop=False then
  begin
    if FileExists('test'+nr+'.bat') then
    begin
      ExeAndWait(ExtractFilePath(Application.ExeName) + 'test'+nr+'.bat', SW_SHOWNORMAL);
    end;
    Timer2.Enabled := True;
  end;
end;




procedure TForm1.Timer3Timer(Sender: TObject);
var nr:string;
begin
  nr := '3';
  Timer3.Enabled := False;

  if g_stop=False then
  begin
    if FileExists('test'+nr+'.bat') then
    begin
      ExeAndWait(ExtractFilePath(Application.ExeName) + 'test'+nr+'.bat', SW_SHOWNORMAL);
    end;
    Timer3.Enabled := True;
  end;
end;

Answer 1

Using a timer isn't the way to go here since you want to run lengthy operations in the background. The way you have it currently, running the batch files will block your user ui if the batch files take some time.

Better approach: Use explicit threads. Create your own thread class as a descendant of TThread. Each instance of your class is given a specific file name that it will take care of that it is run continuously.

unit BatchExecutionThread;

interface
uses Classes;

type

TBatchExecutionThread = class (TThread)
  private
    pBatchFileToExecute: string;
  public
    constructor Create(ABatchFileToExecute: string);

    procedure Execute; override;
end;

implementation

uses SysUtils, Windows;

constructor TBatchExecutionThread.Create(ABatchFileToExecute: string);
begin
  inherited Create;

  pBatchFileToExecute := ABatchFileToExecute;
end;

procedure TBatchExecutionThread.Execute;
begin
   { While no stop requested }
   while(not Terminated) do
   begin
      try
        { Execute Batch file if it exists }
        if FileExists(pBatchFileToExecute) then
        begin
          ExeAndWait(pBatchFileToExecute, SW_SHOWNORMAL);
        end;
      except
        { Ignore exception }
      end;
      { Wait a second }
      Sleep(1000);
   end;
end;

end.

Then, from your application, you can create a whole lot of different instances of this thread class for each of the file you want to run.

For example like this:

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages, System.SysUtils, System.Variants, System.Classes, Vcl.Graphics,
  Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls, BatchExecutionThread, System.Generics.Collections;

type
  TForm1 = class(TForm)
    ButtonStart: TButton;
    ButtonStop: TButton;
    procedure ButtonStartClick(Sender: TObject);
    procedure ButtonStopClick(Sender: TObject);
  private
    pThreads: TList<TBatchExecutionThread>;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.ButtonStartClick(Sender: TObject);
var
  i: Integer;
  FileName: string;
  Thread: TBatchExecutionThread;
begin
  if(pThreads = nil) then
  begin
    { Create a list where we store the running threads }
    pThreads := TList<TBatchExecutionThread>.Create;

    { Create 10 threads with the batch file names from 1 to 10 }
    for i:= 1 to 10 do
    begin
      { Build the filename }
      FileName := ExtractFilePath(Application.ExeName) + 'test'+ i.ToString() +'.bat';

      { Create a thread for this file }
      Thread :=  TBatchExecutionThread.Create(FileName);
      Thread.FreeOnTerminate := true;

      { Add the thread to the list }
      pThreads.Add(Thread);

      { Start the thread }
      Thread.Start();
    end;
  end;
  { else Already started }
end;

procedure TForm1.ButtonStopClick(Sender: TObject);
var
 Thread: TBatchExecutionThread;
begin
  if(pThreads <> nil) then
  begin
    { Tell all threads to stop }
    for Thread in pThreads do
    begin
      Thread.Terminate;
    end;

    { Delete list of threads }
    FreeAndNil(pThreads);
  end;
  { else not started yet }
end;

end.

Answer 2

The accepted answer already outlines that timers are not the solution to your problem, and you should be using threading instead. However, I think there are more fundamental issues with the code you posted.

One important principle of software engineering is called DRY, short for Don't repeat yourself. Whenever you find yourself doing the same thing over and over again, chances are you're doing it wrong. But you're already on the right track with suggesting to use a loop rather than creating 32 timers individually.

It looks like you wrote a separate procedure for every timer, repeating the same code with very minor differences. Instead, you want to write the procedure only once, and assign it to all the timers. The only adjustment you have to make is determining the number for the filename from code.

procedure TForm1.TimerTimer(Sender: TObject);
  var FileName: string;
begin
  FileName := ExtractFilePath(Application.ExeName) + 'test' + String(TTimer(Sender).Name).Replace('Timer', '') + '.bat';
  TTimer(Sender).Enabled := False;

  if (not g_stop) then
  begin
    if FileExists(FileName) then
    begin
      ExeAndWait(FileName, SW_SHOWNORMAL);
    end;
    TTimer(Sender).Enabled := True;
  end;
end;

Notice that I'm also saving the filename to a variable, rather than constructing it two times, as in your code. That's the same principle - you don't want to do things twice. Imagine you wanted to change the way the files are named - with your original version, you'd have to adjust your code in 64 places (twice for every timer), now you would need to make only one adjustment.

In the same vein, you could create all your timers in a loop from code. Note again that this is probably not a good solution for your problem, but it's a good exercise non the less.

procedure TForm1.CreateTimers;
  var i: integer;
      Timer: TTimer;
begin
  for i := 1 to 32 do
  begin
    Timer := TTimer.Create(self);
    Timer.Interval := 1000;
    Timer.Name := 'Timer' + i.ToString;
    Timer.OnTimer := TimerTimer;
  end;
end;