The Wiert Corner – irregular stream of stuff

Jeroen W. Pluimers on .NET, C#, Delphi, databases, and personal interests

  • My badges

  • Twitter Updates

  • My Flickr Stream

  • Pages

  • All categories

  • Enter your email address to subscribe to this blog and receive notifications of new posts by email.

    Join 1,994 other followers

Delphi: do not do “if (not DirectoryExists(path)) then ForceDirectories(path))”

Posted by jpluimers on 2013/07/26

During code reviews, I often see people do things like this:

if (not DirectoryExists(Path)) then
  ForceDirectories(Path))

or this:

if (not TDirectory.Exists(Path)) then
  TDirectory.CreateDirectory((Path))

Half a year ago, I wrote about .NET/C#: do not do “if (!Directory.Exists(path)) Directory.CreateDirectory(path))”.

The same holds for since Delphi XE introduced the ForceDirectories method in the SysUtils and FileCtrl units and the TDirectory.CreateDirectory method in the IOUtils unit.

You don’t need the if statements here in Delphi either: The methods ForceDirectories and TDirectory.CreateDirectory (that internally calls ForceDirectories) will do nothing if the directory already exists

So your code only needs to be like this:

ForceDirectories(Path));

or this:

TDirectory.CreateDirectory((Path));

This is how a simplified ForceDirectories method implementation looks like:

function ForceDirectories(Dir: string): Boolean;
begin
  Result := True;
  if Dir = '' then
    raise EInOutError.CreateRes(@SCannotCreateDir);
  Dir := ExcludeTrailingPathDelimiter(Dir);
  if (Dir = '') or DirectoryExists(Dir) then Exit;
  Result := ForceDirectories(ExtractFilePath(Dir)) and CreateDir(Dir);
end;

–jeroen

via: .NET/C#: do not do “if (!Directory.Exists(path)) Directory.CreateDirectory(path))” « The Wiert Corner – irregular stream of stuff.

16 Responses to “Delphi: do not do “if (not DirectoryExists(path)) then ForceDirectories(path))””

  1. Dennis said

    I really don’t see *any* problem in writing such code. It works and makes the code more readable and isn’t a big performance hog (you’re not calling this in a loop, aren’t you?). The really interessting part is the UNC problem which Marius showed to you – that’s really important while using DirectoryExists() and not if you add this useless if-statement…

    • jpluimers said

      Less code means less clutter, and less chances to introduce mistakes. And I think DRY applies here as well: it is a repetition you don’t need.
      Note that the `DirectoryExists` bug is fixed in Delphi XE2 and up.

  2. It’s a bit weak that the library code calls DirectoryExists at all. Surely it should try to create the directory every time. And then gracefully handle the error that occurs if the directory already exists. The system will also be checking whether or not the directory exists. Your post then becomes, don’t check three times, check only twice. ;-)

    • jpluimers said

      I’m not sure if the smiley is for your whole comment, so I regard it as a joke.

      Remember the case for slow Delphi IDE startup? They used to load all module translations in a loop containing try-except blocks for every load. Replacing that with a check if the translated module existed before loading it caused a large performance gain.

      Error handling (both exceptions and in the Windows API) can be really slow. Not as slow as a failed module load, but still noticeable. So I understand the DirectoryExist call there.

  3. salvadorbs said

    Nice post. Thank you. :)

  4. I find such cases all over the place too :-)

    You should write a follow-up on how to get rid of those too.

    I usually use a macro like this:
    1) Perform a search for “if (not DirectoryExists(”
    2) Start recording
    3) CTRL + Y (to delete line)
    4) DELETE + DELETE (to inindent the line below)
    5) CTRL + F (to jump to next instance)
    6) Stop recording

    The reason behind the search at the _end_ of the macro script is that I can verify that I really want to perform the editing of these two lines…
    If “yes”: CTRL+SHIFT+P (to execute the macro)
    If “no”: CTRL+L (to move to next instance

    I’m not sure if this is the best way, and it’s not multi-unit. Each unit must be opened individually.

  5. Marius said

    Watch out with UNC paths as te DirectoryExists() function is broken in the XE release..

    http://qc.embarcadero.com/wc/qcmain.aspx?d=97555
    SysUtils.DirectoryExists returns True for non-existing UNC paths

  6. Guess you need to fix the final version of the code… as it reads like the “broken” one…

  7. EMB said

    Ok. You say to don’t use:
    if (not DirectoryExists(Path)) then
    ForceDirectories(Path))

    and instead use:
    if (not DirectoryExists(Path)) then
    ForceDirectories(Path))

    Wait… what?

    • jpluimers said

      Sorry for the mishap. This seems to be one of the few cases where I the WordPress “bogus autosave message” I posted about here http://en.forums.wordpress.com/topic/bogus-view-autosave-message?replies=3#post-1304197 wasn’t so bogus.

      So the autosave had the code fragements different, but since the autosave message is bogus 99% of the times, I ignored it. That was mistake 1. Normally not fatal. But mistake 2 was fatal: not checking if the scheduled blog post was in fact completely correct.

      Thanks for all the notifications: I now fixed the code. Again (:

      –jeroen

  8. Rob said

    I don’t see a difference between the “good” and “bad” code….

  9. Jürgen Krämer said

    I guess you wanted to write the 3rd and 4th example without the if statements.

  10. Matthias said

    And what’s the difference?

    if (not DirectoryExists(Path)) then
    ForceDirectories(Path))

    looks like

    if (not DirectoryExists(Path)) then
    ForceDirectories(Path))

    Simply used copy & paste?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.

 
%d bloggers like this: