Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
2  
The more serious problem I see is one with DeleteFile, namely that its name tells you nothing about the return value. Ignoring that and assuming a reasonable name, I don't see how 4 is unreadable. The assignment doesn't have any side effect (apart from changing the variable, of course). –  delnan Sep 19 '11 at 17:00
    
@delnan: suggestions? I thought it would be safe to assume that such a function reports success or failure. My point was that the side-effect of determining if a file is deleted, is actually deleting it. –  thoiz_vd Sep 19 '11 at 17:04
add comment

5 Answers

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.
}
share|improve this answer
    
Is it realistic to think that it is always predictable if something can be done? After all this is just one specific example. –  thoiz_vd Sep 19 '11 at 17:06
    
@thoiz_vd: The idea is to make exceptional situations exceptional, and normal situations visible in the code. If DeleteFile() should always work, then why return a true/false value at all? If it is not completely abnormal for DeleteFile() to fail, then this pattern should work well. –  John Fisher Sep 19 '11 at 18:20
    
This is usually a bad idea: now you have two code paths (if CanDeleteFile returns false, and if DeleteFile throws an exception) for one logical situation (the file could not be deleted). And you must handle the possible exception from DeleteFile, because your application is running concurrently with other applications that can modify the file-system (for example, another application could rename the file between your call to CanDeleteFile and DeleteFile). –  John Bartholomew Sep 19 '11 at 18:21
    
@John Fisher: Whether failure to delete a file is exceptional depends on the context. I believe a better suggestion is to have two functions: TryDeleteFile that returns a success/failure value, and DeleteFile which returns nothing but throws an exception on failure. Then you can use whichever is appropriate in the context, which is not something that is known by those functions. –  John Bartholomew Sep 19 '11 at 18:24
    
@JohnBartholomew: Look at my edits. Also, consider the common patterns in the .NET framework around this exact type of operation. (File.Exists(), File.Delete(), and others). –  John Fisher Sep 19 '11 at 18:27
show 3 more comments

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.

share|improve this answer
    
-1 Please don't promote excessive exception handler bloat. –  Craig Young Oct 15 '13 at 8:56
add comment

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

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
1  
Odd, then, that the SysUtils.DeleteFile function in Delphi returns a boolean in exactly the way that the OP suggests. And clearly the System.Random function can't be much use if it returns the same value every time. –  John Bartholomew Sep 19 '11 at 20:10
    
Of course, it's not a hard and fast rule. But in general, this is one of the primary differences between functions and procedures. Others being that functions do not mutate their inputs, and procedures do not have a return value. In fact, your SysUtils.DeleteFile example is a better function, as it has documentation listing exactly what it does, and noting that file existence is an implicit input. –  Spencer Rathbun Sep 19 '11 at 20:25
    
Specifically, SysUtils.DeleteFile returns function sucess or failure, not whether the file got deleted. You can imply that from True, but not from False, which could be misleading. –  Spencer Rathbun Sep 19 '11 at 20:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.