11

This is how I am trying to check if I can read the file before actually reading it

FileStream stream = new FileStream();
try
{
   // try to open the file to check if we can access it for read
   stream = File.Open(this.DataSourceFileName, FileMode.Open, FileAccess.Read);
}
catch (IOException ex)
{
   return false;
}
finally
{
   stream.Dispose();
}

Is this the right way?

Also is File.Open similar to File.ReadAllText, what I mean is, are they equally expensive for performance?

Pawan Nogariya
  • 8,330
  • 12
  • 52
  • 105
  • 1
    possible duplicate of [how can you easily check if access is denied for a file in .NET?](http://stackoverflow.com/questions/265953/how-can-you-easily-check-if-access-is-denied-for-a-file-in-net) – Alex Jun 26 '13 at 11:21
  • check this answer http://stackoverflow.com/a/876513/1129995 – Zaki Jun 26 '13 at 11:22
  • File.Open creates a FileStream object which you can then use to access the content. File.ReadAllText actually starts the streams processing of it internally. So yes, File.ReadAllText is more expensive. – Lorcan O'Neill Jun 26 '13 at 11:22
  • Check this question http://stackoverflow.com/questions/876473/is-there-a-way-to-check-if-a-file-is-in-use Also use `using` – Dave Williams Jun 26 '13 at 11:22
  • `File.Open` and `File.ReadAllText` do different things. One just opens a file, and the other opens it and does an arbitrary amount of additional work so comparing them is not helpful. Maybe you can clarify what you mean. – bmm6o Jun 26 '13 at 16:18

3 Answers3

11

Whether a file can be read depends on a number of factors: do you have permissions, whether the hard disk is broken. I would probably have gone the same route as you did.

However, you do have to keep in mind that the information you get from this method is just a snapshot. If immediately after you call this method, someone changes the permissions on the file, accessing the file later in your code will still fail. You should not depend on the result of this method.

Just a suggestion, the following code does the same but is a bit more concise:

try
{
    File.Open(this.DataSourceFileName, FileMode.Open, FileAccess.Read).Dispose();
    return true;
}
catch (IOException)
{
    return false;
}

Since you're not really using the stream, you don't have to hold on a reference to it. Instead, you can just immediately dispose of the stream by calling dispose on the result of File.Open().

EDIT:

See https://gist.github.com/pvginkel/56658191c6bf7dac23b3893fa59a35e8 for an explanation on why I've put the Dispose() at the end of the File.Open() instead of using the using statement.

Pieter van Ginkel
  • 29,160
  • 8
  • 71
  • 111
  • Calling `.Dispose()` at the end of a single line can cause resource leaks. You should use a using block instead to ensure it is closed when there is an exception. Otherwise the result of `File.Open()` may not dispose. – NightOwl888 Jul 04 '17 at 17:46
  • 2
    Nope, the code actually is correct. See https://gist.github.com/pvginkel/56658191c6bf7dac23b3893fa59a35e8 for an explanation why. You're right in that you could use a `using`, but the above code does exactly the same, and is shorter. The reason the code can't create a resource leak is that when an exception occurs in `File.Open()`, you won't be able to access the return value anyway because if a method throws, it doesn't return anything. So, it becomes the responsibility of the `File.Open()` method to clean up when it throws. – Pieter van Ginkel Jul 05 '17 at 06:45
  • Since your gist doesn't throw any exceptions, it is not a valid test. If an exception is thrown on line 48, line 49 doesn't execute and therefore the stream is not disposed. The same would be the case on line 54 - `Dispose()` is not called when the file doesn't exist or the user doesn't have permission. A using or a finally block are the only 2 ways to guarantee `Dispose()` will be called in the case of an exception. You could change that line to `using (var _ = File.Open(this.DataSourceFileName, FileMode.Open, FileAccess.Read)) {}` and it would be fine. – NightOwl888 Jul 05 '17 at 07:43
  • 1
    Well, that's what I was trying to get at in my comment. If the user doesn't have permission, `File.Open()` will throw an exception. If any method throws an exception, it doesn't return anything, so there is nothing to call `.Dispose()` on. If e.g. you have a method like `public IDisposable Method() { throw new Exception("Hi"); return /* something */; }`, the `return` will never be executed and nothing will be returned. Please have a look at the link about `using` in the Windows documentation. That page explains it better than I can. – Pieter van Ginkel Jul 05 '17 at 07:47
  • Ahh, okay that makes sense. Thanks for the explanation. – NightOwl888 Jul 05 '17 at 07:48
4

If you want to check for exceptions, just add appropriate try..catch to Dan Dinu code e.g.

  try {
    using (FileStream stream = File.Open(this.DataSourceFileName, FileMode.Open, FileAccess.Read)) {
      ... // <- Do something with the opened file
      return true; // <- File has been opened
    }
  }
  catch (IOException) {
    return false; // <- File failed to open
  }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
-1

You solution looks fine. You could also use a "using" statement:

using (FileStream stream = new FileStream()) 
{
       try {
       stream = File.Open(this.DataSourceFileName, FileMode.Open, FileAccess.Read);
       }
        catch { return false; }
}

The compiler translates this into a try/finally block and automatically disposes the object after the block code is executed.

File Open only opens the file for read/write.

ReadAllText opens the file reads the text and closes it so it will take longer.; it;s up to you to choose the method suitable to your case.

Dan Dinu
  • 32,492
  • 24
  • 78
  • 114
  • Thanks! But how will I know if exception occurs, because only then I will be able to decide I can read a file of not and return the result accordingly? – Pawan Nogariya Jun 26 '13 at 11:28
  • @PawanNogariya you can put a try catch statement inside the using statement I edited my response – Dan Dinu Jun 27 '13 at 14:03
  • 5
    This is *not* the way to use `using`. You should not be assigning `stream` within the `using`. – Jim Mischel Jun 27 '13 at 15:19