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