Search code examples
delphidelphi-xe7

How to control execution without lots of IFs?


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.


Solution

  • 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;