replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Based of xDaevax comment
The Stream.CopyTo() method is calling the InternalCopyTo() method which in turn is doing this
byte[] buffer = new byte[bufferSize]; int read; while ((read = Read(buffer, 0, buffer.Length)) != 0) destination.Write(buffer, 0, read);
After discussing these with rolfl I will change the former implementation to
private const int defaultBlockSize = 81920;
public virtual byte[] ReadFileContents(string filePath)
{
return InternalReadFileContent(filePath, defaultBlockSize);
}
private byte[] ReadFileContents(string filePath, int blockSize)
{
return InternalReadFileContent(filePath, blockSize);
}
public virtual byte[] InternalReadFileContent(string filePath, int blockSize)
{
if (!this.FileExists(filePath)) { return null; }
using (FileStream fs = new FileStream(filePath, FileMode.Open))
{
byte[] content = new byte[fs.Length];
using (MemoryStream ms = new MemoryStream(content))
{
fs.CopyTo(ms, blockSize);
return content;
}
}
}
Now
- the
MemoryStreamis initialized with an array which doesn't need to be resized ToArray()which creates a copy of the internal buffer won't be called omitting the copy- the default buffersize of the
Stream.CopyTo(Stream)is used if the overloadedReadFileContents(String)is called. - if one want to use a different block size he/she can still do it
Based of xDaevax comment
The Stream.CopyTo() method is calling the InternalCopyTo() method which in turn is doing this
byte[] buffer = new byte[bufferSize]; int read; while ((read = Read(buffer, 0, buffer.Length)) != 0) destination.Write(buffer, 0, read);
After discussing these with rolfl I will change the former implementation to
private const int defaultBlockSize = 81920;
public virtual byte[] ReadFileContents(string filePath)
{
return InternalReadFileContent(filePath, defaultBlockSize);
}
private byte[] ReadFileContents(string filePath, int blockSize)
{
return InternalReadFileContent(filePath, blockSize);
}
public virtual byte[] InternalReadFileContent(string filePath, int blockSize)
{
if (!this.FileExists(filePath)) { return null; }
using (FileStream fs = new FileStream(filePath, FileMode.Open))
{
byte[] content = new byte[fs.Length];
using (MemoryStream ms = new MemoryStream(content))
{
fs.CopyTo(ms, blockSize);
return content;
}
}
}
Now
- the
MemoryStreamis initialized with an array which doesn't need to be resized ToArray()which creates a copy of the internal buffer won't be called omitting the copy- the default buffersize of the
Stream.CopyTo(Stream)is used if the overloadedReadFileContents(String)is called. - if one want to use a different block size he/she can still do it
- How will this code handle very large files (currently tested up to a file length of 5098)?
Already answered by rolfl.
- Is using the Array.Resize<> method in the byte[] return overload efficient, or is there a better way?
- Is there a way to refactor to improve the readability and / or simplify the code?
As you have a filestream and a defined buffer size you can simply copy the content of the filestream to a memory stream. You can then turn this memory stream into a byte array.
After implementing the guard conditions and buffersize like rolfl mentioned your former methods will look like
public virtual byte[] ReadFileContents(string filePath)
{
if (!this.FileExists(filePath)) { return null; }
using (FileStream fs = new FileStream(filePath, FileMode.Open))
{
int bufferSize = (int)Math.Min(1024 * 1024, fs.Length);
using (MemoryStream ms = new MemoryStream(bufferSize))
{
fs.CopyTo(ms, bufferSize);
return ms.ToArray();
}
}
}
public virtual string ReadFileContents(string filePath, Encoding fileEncoding)
{
string returnValue = string.Empty;
byte[] content = ReadFileContents(filePath);
if (content == null) { return String.Empty; }
return fileEncoding.GetString(content);
}
lang-cs