How to control execution without lots of IFs?

Question

I have a process that starts with importing data from file and then executes a series of procedures, but at anytime it can find a problem and should stop executing rest of them and run another set.

Here is my example where every procedure sets global gStop variable that indicates stopping the process. If it was stopped, I need run some code for that at the end.

var gStop:boolean;


procedure Run;
begin
  gStop:=False;
  Import; // imports data from file
  If Not gStop Then
    AfterImport1;
  If Not gStop Then
    AfterImport2;
  If Not gStop Then
    AfterImport3;
  If Not gStop Then
    AfterImport4;
  If Not gStop Then
  If fTypeOfData = cMSSQL Then // function returns type of imported data
  begin
    ProcessMSSQLData1;
    If not gStop Then
      ProcessMSSQLData2;
    If not gStop Then
      ProcessMSSQLData3;
    If not gStop Then
      If fObjectAFoundInData Then // function checks if ObjectA was found in imported data
        ProcessObjectA;
    If not gStop Then
      ProcessMSSQLData4;
  end;
  If Not gStop Then
    AfterImport5;
  ...
  // If stopped at anytime
  If gStop then
  begin    
    LogStoppedProcess; 
    ClearImportedData;
    ...  
  end;
end;

In my case it actually goes over 200 lines of code, so I have to scroll up and down when I maintain this part of code.

Any way I can improve this procedure to be more readable, easier to maintain or just is there any other way I can stop the process without all the IFs?

EDIT 1:

Every procedure can find a faulty data and can set gStop := True; and the process should skip all the rest of the procedure and just execute the part of the code at the end when gStop = True;

EDIT 2:

I would like to keep the workflow controlled from main procedure (Run) so I can see all the tasks that are run after the main Import. I only see more confusion and less readability and maintainability if I break the execution into many smaller procedure. Then I could just have:

procedure Run;
begin
  gStop:=False;
  Import; // imports data from file
  RunEverytingAferImport; // execute ALL tasks after import
  // If stopped at anytime
  If gStop then
  begin    
    LogStoppedProcess; 
    ClearImportedData;
    ...  
  end;
end;

This workflow doesn't seem designed correctly. I want to know what are main tasks running after Import and not go on discovery trip every time I need to review it. All tasks are already group into the procedure by purpose, what they do and how they do it, and results.

CONCLUSION:

Even though not the best option, I decided to go with Raising Exception when stopping the process is needed. I 'kind of' understand the implications that this approach brings (more will be known once I implement it), but it seems a logical step towards some-day better implementation of the whole process. Now I will not see all these IFs for every task execution. Good! The code will be more readable, maintainable.

I read the links provided to explain the pitfalls of using Exceptions in stopping workflow execution, but, as Dalija Prasnikar explained in comments, this is not a performance related part of the task; process is only executed once per application run; tasks are already quite structured by what they do; tasks already include multiple IF statements where stopped process is checked and so on, so I think exceptions don't fall into a really bad solution to my problem.

Also, if I turn tasks into functions with returning results, I think I will have the same problem, checking values of every task and stop or continue the process based on that.

So, Raising exception is what I choose.

Answer 1

You should use custom exception and raise it when ever you encounter reason to break your work and go to Stopped code. In your Import, AfterImport1 and other code logic just call Stop procedure and it will execute Stopped procedure. On the other hand if all goes well, Stopped will not be called.

You can derive your exception from EAbort creating silent exception

type
  EStopException = class(EAbort);

or derive from base Exception class to use regular type exception.

type
  EStopException = class(Exception);

procedure Stop(const Msg: string);
begin
  raise EStopException.Create(Msg);
end;

procedure Import;
var sl: TStringList;
begin
  sl := TStringList.Create;
  try
    // your code logic
    if NeedToStop then Stop('something happened');        
  finally
    // perform any cleanup code needed here
    sl.Free;
  end;
end;

procedure Stopped;
begin
end;

procedure Run;
begin
  try
    Import;
    AfterImport1;
    AfterImport2;
  except
    on e: EStopException do
      Stopped;
  end;
end;

Answer 2

Similar to RTTI-based example of Jens Borrisholt, but without RTTI. And thus not bound to a single super-object containing all methods.

type TAfterImportActor = reference to procedure (var data: TImportData; var StopProcess: boolean);
     TAfterImportBatch = TList< TAfterImportActor >;

var Batch1, Batch2, BatchMSSQL: TAfterImportBatch; // don't forget to create and free them.

procedure InitImportBatches;
begin
  Batch1 := TAfterImportBatch.Create;
  Batch2 := TAfterImportBatch.Create; 
  BatchMSSQL := TAfterImportBatch.Create;

  Batch1.Add( AfterImport1 );
  Batch1.Add( SomeObject.AfterImport2 ); // not only global procedures
  Batch1.Add( SomeAnotherObject.AfterImport3 ); // might be in different modules
  Batch1.Add( AfterImport4 );

  Batch2.Add( AfterImport5 );
...
  Batch2.Add( AfterImport123 );

  BatchMSSQL.Add( ProcessMSSQLData1 );
...
  BatchMSSQL.Add( ProcessMSSQLData5 );
end;

procedure ProcessBatch(const Batch: TAfterImportBatch; var data: TImportData; var StopProcess: Boolean);
var action: TAfterImportActor;
begin
  if StopProcess then exit;

  for action in Batch do begin
    action( data, StopProcess );
    if StopProcess then break;
  end; 
end;

procedure Run;
var gStop: boolean;
    data: TImportData;
begin
  gStop:=False;
  Import(data, gStop); // imports data from file

  ProcessBatch( Batch1, data, gStop );

  If fTypeOfData = cMSSQL Then // function returns type of imported data
     ProcessBatch( BatchMSSQL, data, gStop );

  ProcessBatch( Batch2, data, gStop );
  ...

  // If stopped at anytime
  If gStop then
  begin    
    LogStoppedProcess; 
    ClearImportedData;
    ...  
  end;
end;

PS. This framework ( and RTTI framework above ) lack any exception control, so if any of import processors would raise some not-caught exception - the execution would jump out of the main process loop without calling clean-up routines. So that means you still have to caught possible exceptions within every actor (fragile) or within the Run procedure. But in the latter case you might totally omit gStop variable and instead raise your custom exception. Personally i'd prefer the exception-based way to Boolean flag. And even EurekaLog might be of use, if your failing afterimport procedure would add to the exception some meaningful message as to why exactly the import was aborted.

PPS. I'd also split gStop into TWO different variables/exceptions: Batch Cancel and Import Abort ones. Then the If fTypeOfData = cMSSQL Then - or any other prerequisite - check could be just the first actor in the batch. Then the batches could be just combined into 2-nd tier array/collection.

I also think EurekaLog would ignore your custom exceptions would you inherit them from EAbort - http://docwiki.embarcadero.com/RADStudio/XE8/en/Silent_Exceptions

type TAfterImportActor = reference to procedure (var data: TImportData; var CancelBatch, AbortImport: boolean);
     TAfterImportBatch = TList< TAfterImportActor >;

var Batch1, Batch2, BatchMSSQL: TAfterImportBatch; 
// don't forget to create and free them.

    ImportBatches: TArray<TAfterImportBatch>;

procedure MSSQLCheck(var data: TImportData; var CancelBatch, AbortImport: boolean);
begin
  CancelBatch := data.fTypeOfData <> cMSSQL; 
end;

procedure InitImportBatches;
begin
  Batch1 := TAfterImportBatch.Create;
  Batch2 := TAfterImportBatch.Create; 
  BatchMSSQL := TAfterImportBatch.Create;

  Batch1.Add( AfterImport1 );
  Batch1.Add( SomeObject.AfterImport2 ); // not only global procedures
  Batch1.Add( SomeAnotherObject.AfterImport3 ); // might be in different modules
  Batch1.Add( AfterImport4 );

  Batch2.Add( AfterImport5 );
...
  Batch2.Add( AfterImport123 );

  BatchMSSQL.Add( MSSQLCheck ); // If fTypeOfData = cMSSQL Then Run This Batch
  BatchMSSQL.Add( ProcessMSSQLData1 );
...
  BatchMSSQL.Add( ProcessMSSQLData5 );

  ImportBatches := TArray<TAfterImportBatch>.Create
     ( Batch1, BatchMSSQL, Batch2);
end;

procedure ProcessBatch(const Batch: TAfterImportBatch; var data: TImportData; var StopProcess: Boolean);
var action: TAfterImportActor; CancelBatch: boolean;
begin
  if StopProcess then exit;

  CancelBatch := false;
  for action in Batch do begin
    action( data, CancelBatch, StopProcess );
    if StopProcess or CancelBatch then break;
  end; 
end;

procedure Run;
var gStop: boolean;
    data: TImportData;
    CurrentBatch: TAfterImportBatch;
begin
  gStop := False;
  Import(data, gStop); // imports data from file

  for CurrentBatch in ImportBatches do begin   
    if gStop then break; 
    ProcessBatch( CurrentBatch, data, gStop );
  end;

  ...

  // If stopped at anytime
  If gStop then
  begin    
    LogStoppedProcess; 
    ClearImportedData;
    ...  
  end;
end;

PPPS. You would also maybe want to have a look at http://www.uweraabe.de/Blog/2010/08/16/the-visitor-pattern-part-1/

Focusing how different action-makers are registered and invoked. This might give you some ideas, though it is not exactly your problem.

Another thing to consider might be Multi-cast events like those in Spring4D library.

Answer 3

The best way to do this is via RTTI

The following is a dummy implementaiton of you problem: unit ImportU;

interface

{$M+}

uses
  RTTI;

Type
  TImporter = class
  strict private
    RttiContext: TRttiContext;
    gStop: Boolean;
    function GetMethod(const aMethodName: string): TRttiMethod;
    procedure Import;
  public    
    procedure AfterImport1;
    procedure AfterImport2;
    procedure AfterImport3;
    procedure AfterImport4;
    procedure Run;
  end;

implementation

uses
  Sysutils;
{ TImporter }

procedure TImporter.AfterImport1;
begin

end;

procedure TImporter.AfterImport2;
begin

end;

procedure TImporter.AfterImport3;
begin
  gStop := True;
end;

procedure TImporter.AfterImport4;
begin

end;

function TImporter.GetMethod(const aMethodName: string): TRttiMethod;
begin
  Result := RttiContext.GetType(Self.ClassType).GetMethod(aMethodName);
end;

procedure TImporter.Import;
begin

end;

procedure TImporter.Run;
var
  i: Integer;
  Stop: Boolean;
  RttiMethod: TRttiMethod;
begin
  i := 0;
  repeat
    inc(i);
    RttiMethod := GetMethod('AfterImport' + IntToStr(i));

    if RttiMethod = nil then
      break; //Break loop

    RttiMethod.Invoke(self, []);
  until (gStop = false);
end;

end.

This implementation has the advantage that if you create a AfterImport function it will automatically get called.

Answer 4

Why not to divide procedure into smaller subprocedures? For example:

var gStop:boolean;

procedure AfterImport;
begin
  If Not gStop Then
    AfterImport1;
  If Not gStop Then
    AfterImport2;
  If Not gStop Then
    AfterImport3;
  If Not gStop Then
    AfterImport4;

end;

procedure ProcessMSSQLData;
begin
  If Not gStop Then
  If fTypeOfData = cMSSQL Then // function returns type of imported data
  begin
    ProcessMSSQLData1;
    If not gStop Then
      ProcessMSSQLData2;
    If not gStop Then
      ProcessMSSQLData3;
    If not gStop Then
      If fObjectAFoundInData Then // function checks if ObjectA was found in imported data
        ProcessObjectA;
    If not gStop Then
      ProcessMSSQLData4;
  end;
end;

procedure AfterProcessMSSQLData;
begin
  If Not gStop Then
    AfterImport5;  
end;

In this way, your final Run; will have like 15 lines of code:

procedure Run;
begin
  gStop:=False;
  Import; // imports data from file
  AfterImport;
  ProcessMSSQLData;
  AfterProcessMSSQLData;
  // If stopped at anytime
  If gStop then
  begin    
    LogStoppedProcess; 
    ClearImportedData;
    ...  
  end;
end;

A lot more readable and a bit easier to maintain.