3

What is the proper approach to functions that have side-effects in Delphi/Pascal?

For example, I could have a boolean function DeleteFile that returns True if the file was deleted and False otherwise. How do I collect the return value?

1) The very explicit form would be:

if DeleteFile(MyFilePath) then
begin
  IsMyFileDeleted := True;
end;
else
begin
  IsMyFileDeleted := False;
end;

2) Less bulky would be:

IsMyFileDeleted := False;
if DeleteFile(MyFilePath) then
begin
  IsMyFileDeleted := True;
end;

3) Or even shorter, but discouraged by JEDI - perhaps it is acceptable just to collect return values:

IsMyFileDeleted := False;
if DeleteFile(MyFilePath) then IsMyFileDeleted := True;

My problem with 2 and 3 is that I explicitly state something (X := False) that I may not know for sure.

4) I would rather not set the variable beforehand and have its value completely determined by the function:

IsMyFileDeleted := DeleteFile(MyFilePath);

Here the issue is that it is a bit misleading, because assigning a value to a boolean has a pretty serious side-effect. I'm not sure if this usage is desirable.

Some argue that functions should never have side-effects, but I think some of Pascals internal functions violate that.

5) The alternative would be making it procedures with a result variable to be filled by the procedure:

DeleteFile(MyFilePath, IsMyFileDeleted);

That might go against the trend to keep parameter use to a minimum (Clean Code).

4 and 5 would be the only ways to collect a return value other than boolean, I suppose? Does that mean anything for boolean functions?

Yes, there are many ways to do the same thing and maybe I'm just worried about nothing really, but I'm trying to develop a good style and I wonder if there are any best practices. For a beginner, many principles seem to be in conflict all the time.

gnat
  • 20,543
  • 29
  • 115
  • 306
thoiz_vd
  • 131

5 Answers5

11

Option #4 is fine, really. Delphi isn't a functional programming language and doesn't really try to be one, so talking about "side effects" in this context is kind of silly.

Deleting the file isn't "a side effect" of calling DeleteFile, it's simply the effect of calling it. You don't call it to get a boolean, you call it to delete a file. If anything, the "side effect" is returning a result at all.

Mason Wheeler
  • 83,213
4

I think you are making things way too hard for yourself.

There are many functions in the VCL and the Windows API that are only functions because they return success or failure and only functions can return anything. To all intents and purposes though these functions are procedures in what they do.

If they didn't return their success or failure as a function result, you would have to jump through hoops to check whether they were successful, or would have to rely on them throwing exceptions if they just weren't able to do what they are meant to do but didn't encounter any real problems:

try
  DeleteFile(MyFilePath);
  IsMyFileDeleted := True;
except
  IsMyFileDeleted := False;
end;

You really don't want to litter your code with constructs like that. Plus, especially on win32, exceptions are expensive. And, using exceptions for flow control is not good practice anyway.

So, use #4

IsMyFileDeleted := DeleteFile(MyFilePath);

It does exactly what you need and it is pretty obvious that you are just capturing the success or failure of the DeleteFile.

And if you have a "function that really is a procedure reporting success or failure" through error codes, you could also do something like:

IsThisSuccessful := (ProcedureAsFunction = S_OK);

Marjan Venema
  • 8,171
2

In delphi, one of the primary differences between procedures and functions is that a function always has a return value, and for each input, you will get one and only one return value out, no matter how many times you run the function on the same input.

So, for your code, #4 is fine, so long as it returns the same value on every run of the same input. So if you run the delete function on a file and return true, then rerun the function on the same file, it should return true again. Even though the file is already gone! The current runtime state should have no impact on your return values in delphi functions. Unless this is implied by the function use states and/or documentation. For a good example, see John's comment below.

If the current state is important, then you should be using a procedure which mutates an input boolean variable, or better, a record that can return more information, such as whether the file exists, and if we have permission to delete.

Or, you should add inputs to your function, to allow for more variation. A good one would be what to do if the file doesn't exist. Finally, if you just want to include current state implicitly, make sure you document that fact.

Spencer Rathbun
  • 3,576
1

Use #4,

IsMyFileDeleted := DeleteFile(MyFilePath);

Or possibly, since this is Delphi,

IsMyFileDeleted := false;
try 
   IsMyFileDeleted := DeleteFile(MyFilePath);
except
   ...
end;

If you want to be properly paranoid about it, if you are concerned an exception might be thrown. I dont think you are achieving anything in the other options other than making simple code unnecessarily complicated.

GrandmasterB
  • 39,412
0

To be clearer about what's going on, you should probably do something like this:

IsMyFileDeleted := False;
if (CanDeleteFile(MyFilePath)) then
begin
    DeleteFile(MyFilePath);
    IsMyFileDeleted := True;
end

The first line is true at the point it has been run (unless the file doesn't exist).

Using CanDeleteFile(...) allows you to separate the true/false features of the code's intent from the exceptional behavior of being unable to delete something that should have been deleted. So, if you can delete the file, then DeleteFile should raise an exception if it can't.

Edited

If I were writing in C#, the first situation would have looked like this. Using exception handling would likely be better in Delphi/Pascal, if available. In this model, DeleteFile() returns nothing at all. Either it works, or it supplies information about why it didn't work.

try
{
    DeleteFile(MyFilePath);
}
catch (IOException ex)
{
    // Do whatever you would have done if DeleteFile() returned false.
}
John Fisher
  • 1,795