Best practice: Using redundant methods from external libraries [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 8 years ago.

Improve this question

In the code base I'm working on there are several examples of

if (!Directory.Exists(dir))
{
    Directory.CreateDirectory(dir);
}

According to the MSDN documentation (http://msdn.microsoft.com/en-us/library/54a0at6s(v=vs.110).aspx) this is redundant because createDirectory won't overwrite an existing directory.

This could be seen as making the code clearer, as it's not obvious from the .CreateDirectory(dir) method that this is the behaviour.

On the flip side, this is code bloat and keeping it around (even adding it to a library/utility class) has its issues (means you have to read/maintain more lines of code for example).

What's considered best practice here?

Answer 1

It may look redundant, but I can see a reason why someone decided to go that way.

The main difference is:

• Directory.Exists() returns just bool
• Directory.CreateDirectory() returns DirectoryInfo

So even when the directory exists, there is additional work performed to get that DirectoryInfo instance, which may not be necessary at all.

Another thing that come up is the fact, that you have to know that Directory.CreateDirectory does not override the directory if it exists! With additional Directory.Exists call even when someone doesn't know that he can really easily figure out what's going on with this piece of code.

And I don't think there is a best practice here.

Answer 2

Personally, I would normally remove the redundant code.

This could be seen as making the code clearer, as it's not obvious from the .CreateDirectory(dir) method that this is the behaviour.

In general, I'd argue that would be better served by a comment, rather than a redundant code path. Adding extra code to avoid a lack of knowledge seems like a weak reason to include the check.

That being said, there is a potential (very minor) performance gain in avoiding the call to CreateDirectory, as that method will construct a DirectoryInfo instance. In practice, this is most likely "noise" (as IO calls tend to be relatively expensive anyways), so its not something I would factor into the equation unless it proved to be a measured problem.

Answer 3

A race condition could cause the creation of a directory to fail even if the preliminary check has passed. Therefore I consider this code as incorrect and I dissuade you from using it.