CODEWITHSUNDEEP
performancedelphiif-statementtry-except
BogdyBBA
BogdyBBA

Reputation: 101

Is it faster to check a file's existence prior to loading, or to catch the exception when it doesn't exist?

I have been recommended to use the second, try-except variant, but I would also like to know what others think: which procedure of the two below (if any) is more time-efficient?

procedure LoadImage(img: TImage; filename: string);
begin
  if fileexists(filename) then
    img.Picture.Loadfromfile(filename)
  else
    img.Picture.Loadfromfile('default.jpg')
end;

or

procedure LoadImage(img: TImage; filename: string);
begin
  try
    img.Picture.Loadfromfile(filename)
  except
    img.Picture.Loadfromfile('default.jpg')
  end
end;

Upvotes: 10

Views: 1446

Answers (3)

CodesInChaos
CodesInChaos

Reputation: 108830

One problem with your question is that you didn't specify the desired semantics. There are two possible interpretations, and which solution is better, depends on that choice. I assume being unable to load the fallback, is a fatal error.

  1. If the first file does exist, load it, else load the second. If the first file exists, but could not be loaded, show an error.
  2. If the first file cannot be loaded, fallback to the second.

If you want the semantics of 1) your first code is fine.

If you want the semantics of 2), neither is good. The first code doesn't have these semantics because:

  1. It has a race condition. If the image gets deleted between checking the existence of the file and loading it, it'll fail.
  2. If the file exists, but cannot be loaded, an exception will be thrown. This can happen if the file is no valid image, or cannot be opened.

The second one uses exceptions for a common case, which is bad.

So to achieve the second semantics I'd use:

procedure LoadImage(img: TImage; filename: string);
var success:boolean;
begin
  success := false;
  if FileExists(filename)
    then try
      img.Picture.LoadFromFile(filename);
      success := true;
    except
    end
  if not success
    then img.Picture.LoadFromFile('default.jpg');
end;

Checking for existence before opening makes the error case faster, but the success case slower. So which one is faster depends on your usage.

Personally I'd use the third variant over the second, even if the image is only missing occasionally, since I believe a normally operating application should not throw exceptions. I'd only care about the performance issue if benchmarking revealed it to be noticeable.

You should also consider a more targeted exception clause. Blanket catching all exception is bad style. Unfortunately I couldn't find a clear specification which exceptions are thrown by TPicture.LoadFromFile, so I'll leave in the blanket clause for now.

Upvotes: 0

Nick Hodges
Nick Hodges

Reputation: 17138

Forget efficiency. Code readability is way, way more important. Premature optimization is the root of all sorts of evil.

The first one is clear in its intentions. Everyone can easily figure out what is up.

The second one makes me stop and go "What the....?"

You never want your code to cause the second reaction.

Upvotes: 8

Jarek Bielicki
Jarek Bielicki

Reputation: 876

If time efficiency is your only criteria, first one will be faster because exception handling is CPU consuming.

FileExists() uses one WinApi call so its fast but it checks only if file exists. If file exists but its in wrong format or its blocked by other thread you will get unhandled exception.

Upvotes: 4

Related Questions