Code analysis
Let's run through your code bit by bit.
First a note about paths: use verbatim strings to disable escape sequences, so you don't need to escape those slashes: @"C:\myfile.m4a"
instead of "C:\\myfile.m4a"
.
public static bool WriteAaray(string originalFile, List<float> myFloatPoints)
{
originalFile
is a path, not a file (stream), and I'm not sure why it's called 'original', so I would rename it to filePath
.
- I'd also fix the typo in
Aaray
.
FileStream inputFileStream = null;
inputFileStream = new FileStream(originalFile, FileMode.Open);
- The 'input' in
inputFileStream
is misleading: you're not reading from this file, you're writing to it. I'd rename it to just file
.
- There's no need for that first line: just write
var file = new ...
instead.
- File streams are disposable, so it's better to use a
using
statement here.
- Note that the
File
class contains several useful methods, such as File.Open
, or the very convenient File.AppendAllText
.
var length = inputFileStream.Length;
inputFileStream.Seek(length, SeekOrigin.Begin);
- There's no need for that local variable (
length
): just pass file.Length
directly into Seek
.
- Instead of manually seeking, consider opening the file with
FileMode.Append
. Not only does that seek to the end for you, it also creates the file if it doesn't exist yet (in your code that'll throw an exception, which will crash your program because you're not catching it).
StringBuilder mystring = new StringBuilder();
string myImpPoints = "[MYIMPPOINTS]";
mystring.Append(myImpPoints);
- While string builders are often more efficient than string concatenation, in this case you're only concatenating two strings (this header and the result of that
string.Join
call), so a string builder won't help here - it might even be less efficient. Note that string.Join
uses a string builder internally, and with 5000 floats it certainly is useful there.
- Again, no need for a local variable (
myImpPoints
) - just pass it into Append
directly.
var points = string.Join(",", myFloatPoints.Select(f => f.ToString()));
mystring.Append(points);
- There's no need for that
Select
call - one of the overloads of string.Join
takes an IEnumerable<T>
, so you can pass your list of floats in directly.
- On the other hand, this only gives you 7 digits of precision. If that's a problem, then you should keep the
Select
part, but be sure to specify "R"
(round-trip) or "G9"
(recommended for performance reasons) as the to-string format.
- Again, no need for a local variable (
points
).
- Note that formatting and parsing string representations of floats is slower than writing and reading them as binary data.
byte[] byteArray = Encoding.ASCII.GetBytes(mystring.ToString());
inputFileStream.Write(byteArray, 0, byteArray.Length);
inputFileStream?.Close();
- Consider using a
StreamWriter
instead. That takes care of encoding and buffering, so you don't have to mess with string builder buffers and byte arrays and all that.
- With a
using
statement around the file stream, you don't need to call Close
manually here. Either way, you don't need an 'elvis operator' here - inputFileStream
obviously won't be null.
return true;
}
- You're returning a boolean to indicate success, but that's actually somewhat misleading: this method can fail, but instead of returning false (which I would expect, with such a method signature) it'll throw an exception. Either catch it and return false (but then the caller won't know why it failed), or don't return anything and let the method throw (but document it).
Alternatives
5000 floats isn't a whole lot, so unless you need to create a lot of these files, the following code should be fast enough:
File.AppendAllText(filePath, "[MYIMPPOINTS]" + string.Join(",", values), Encoding.ASCII);
If, after actually measuring performance, that turns out to be too slow, then here's what your code could look like with the above notes taken into account:
using (var file = File.Open(originalFile, FileMode.Append, FileAccess.Write))
using (var writer = new StreamWriter(file, Encoding.ASCII)) // buffer size can be adjusted if necessary
{
writer.Write("[MYIMPPOINTS]");
if (values.Count > 0)
writer.Write(values[0]);
for (int i = 1; i < values.Count; i++)
{
writer.Write(',');
writer.Write(values[i]);
}
}