Skip to main content
added 623 characters in body
Source Link
CodesInChaos
  • 2.7k
  • 14
  • 24

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily removeremoves whitespace and nullbytes. I guess this is a workaround for not checking the return value Stream.Read.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

Some otheradditional issues:

  • I don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.
  • Why use naming different from the methods of File? Using familiar names makes it easier to learn your library as somebody who is already familiar with the BCL.
  • You're opening the file for writing when you only read. It's common to only have permission to read a file, not write it. Your method will fail unnecessarily for those.
  • I don't think silently returning nothing if the file doesn't exist is a good idea. That will usually turn a clear exception into a null reference exception. For the rare cases where a caller wants to handle absent files without an error, leave that to the caller.

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily remove whitespace.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

Some other issues:

  • I don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.
  • Why use naming different from the methods of File? Using familiar names makes it easier to learn your library as somebody who is already familiar with the BCL.
  • You're opening the file for writing when you only read. It's common to only have permission to read a file, not write it. Your method will fail unnecessarily for those.
  • I don't think silently returning nothing if the file doesn't exist is a good idea. That will usually turn a clear exception into a null reference exception. For the rare cases where a caller wants to handle absent files without an error, leave that to the caller.

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily removes whitespace and nullbytes. I guess this is a workaround for not checking the return value Stream.Read.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

Some additional issues:

  • I don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.
  • Why use naming different from the methods of File? Using familiar names makes it easier to learn your library as somebody who is already familiar with the BCL.
  • You're opening the file for writing when you only read. It's common to only have permission to read a file, not write it. Your method will fail unnecessarily for those.
  • I don't think silently returning nothing if the file doesn't exist is a good idea. That will usually turn a clear exception into a null reference exception. For the rare cases where a caller wants to handle absent files without an error, leave that to the caller.
added 623 characters in body
Source Link
CodesInChaos
  • 2.7k
  • 14
  • 24

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily remove whitespace.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

I also don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.Some other issues:

  • I don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.
  • Why use naming different from the methods of File? Using familiar names makes it easier to learn your library as somebody who is already familiar with the BCL.
  • You're opening the file for writing when you only read. It's common to only have permission to read a file, not write it. Your method will fail unnecessarily for those.
  • I don't think silently returning nothing if the file doesn't exist is a good idea. That will usually turn a clear exception into a null reference exception. For the rare cases where a caller wants to handle absent files without an error, leave that to the caller.

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily remove whitespace.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

I also don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily remove whitespace.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

Some other issues:

  • I don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.
  • Why use naming different from the methods of File? Using familiar names makes it easier to learn your library as somebody who is already familiar with the BCL.
  • You're opening the file for writing when you only read. It's common to only have permission to read a file, not write it. Your method will fail unnecessarily for those.
  • I don't think silently returning nothing if the file doesn't exist is a good idea. That will usually turn a clear exception into a null reference exception. For the rare cases where a caller wants to handle absent files without an error, leave that to the caller.
Source Link
CodesInChaos
  • 2.7k
  • 14
  • 24

At a glance I see several bugs:

  • You're not checking the return value of Stream.Read. It can be smaller than the length you passed in, but you're assuming it's equal.
  • The calls to Trim arbitrarily remove whitespace.
  • fileEncoding.GetString will not work correctly if a buffer boundary falls in the middle of a codepoint that has been encoded as several bytes.

I also don't see the point of most of your methods. ReadFileContents can be implemented by calling File.ReadAllText and File.ReadAllBytes. Simpler and less buggy. Even if you don't like File.ReadAllText, a TextReader will save you a lot of pain.