1
\$\begingroup\$

I am trying to implement a tiny text file read / write class FileIO in order to make the operations of reading / writing pure text files easily. The usage is like:

FileIO.Instance.FileWrite("TextFile1.txt", "This is a test1.");
FileIO.Instance.FileWrite("TextFile2.txt", "This is a test2.");
Console.WriteLine(FileIO.Instance.ReadTxTFile("TextFile1.txt", System.Text.Encoding.Default));
Console.WriteLine(FileIO.Instance.ReadTxTFile("TextFile2.txt", System.Text.Encoding.Default));

The experimental implementation

The experimental implementation of FileIO class is as below.

class FileIO
{
    public readonly static FileIO Instance = new FileIO();
    
    public void FileWrite(string filename, string inputString)
    {
        FileStream file_stream = new FileStream(filename, FileMode.Append);
        byte[] Input_data = System.Text.Encoding.Default.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }
    
    public void FileWrite(string filename, string inputString, FileMode fileMode)
    {
        FileStream file_stream = new FileStream(filename, fileMode);
        byte[] Input_data = System.Text.Encoding.Default.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }
    
    public void FileWrite(string filename, string inputString, FileMode fileMode, Encoding encoding)
    {
        FileStream file_stream = new FileStream(filename, fileMode);
        byte[] Input_data = encoding.GetBytes(inputString);
        file_stream.Write(Input_data, 0, Input_data.Length);
        file_stream.Flush();
        file_stream.Close();
    }

    public string ReadTxTFile(string filename, Encoding encoding)
    {
        System.IO.StreamReader textreader;
        string inputString;
        inputString = "";
        try
        {
            textreader = new System.IO.StreamReader(filename, encoding);
        }
        catch (Exception)
        {
            return "";
            throw;
        }
        inputString = textreader.ReadToEnd();
        textreader.Close();
        return inputString;
    }
}

If there is any possible improvement about:

  • Performance: including data reading/writing speed things

  • The naming and readability

  • Potential drawbacks of the implemented methods

, please let me know.

\$\endgroup\$
6
  • 3
    \$\begingroup\$ Why not use File.ReadAllText and File.WriteAllText? Also, you use classes that implement IDisposable yet you don't Dispose() of them. And why aren't you encapsulating their uses in a using? \$\endgroup\$
    – BCdotWEB
    Jan 9, 2021 at 21:36
  • \$\begingroup\$ @BCdotWEB Thank you for the comments. Do you mean that Dispose() method should be added? May I update the question? \$\endgroup\$
    – JimmyHu
    Jan 11, 2021 at 0:29
  • 1
    \$\begingroup\$ Don't edit the code in the question but consider that fix to apply. Using objects that implement IDisposable. And yes, methods File.ReadAllText and File.WriteAllText are already exist in .NET. Thus you don't need the code you wrote. Learn what the File class can. \$\endgroup\$
    – aepot
    Jan 11, 2021 at 1:14
  • \$\begingroup\$ @aepot OK, I see. Thank you for the suggestion. \$\endgroup\$
    – JimmyHu
    Jan 11, 2021 at 7:54
  • 1
    \$\begingroup\$ @JimmyHu Whether or not an explicit call to Dispose() is needed depends on the case. The best approach is to use a using: stackoverflow.com/questions/17357258/… \$\endgroup\$
    – BCdotWEB
    Jan 11, 2021 at 9:56

1 Answer 1

4
\$\begingroup\$

Naming

  • FileIO: The File class resides inside the System.IO namespace. FileIO is quite a misfortune chose of name. TextFileWrapper, TextFileUtil or TextFileManager would be a more appropriate name for this in my opinion.
  • FileWrite: The general guidance is to start your methods with a verb. So, WriteFile would be a better name for this.
  • ReadTxtFile: This name is a bit misleading. txt can be considered as a file extension, so the caller may no specify the filename as you expect.
  • inputString: Please try to avoid Hungarian notation.

Please try to chase consistency (or symmetry).
Try to name your Read-Write method pair in the same fashion:

  • ReadTextFile, WriteTextFile
  • ReadFile, WriteFile
  • or simply just Read and Write

Stuttering:

  • Maybe it's just for me, but FileIO.Instance.FileWrite seems too repetitive.
  • TextFileManager.Write seems way more natural for me.

Robustness

Your implementation is quite naive. It does not deal with a lot of different cases:

  • What if the fileName points to a non-existing file?
  • What if the inputString is enormously large?
  • What if one of the parameters is null?
  • What if the file has been opened by another method and has not been closed yet?

Your implementation also relies on Encoding.Default, which is computer specific.
Which means you can end up in the following situation:

  • FileA.txt has been written to the disk on MachineA
  • FileA.txt has been copied from MachineA to MachineB
    • where the default encoding is different
  • MachineB could not read FileA.txt

Thread-safety

  • Your instance property does not make your class a singleton.
  • If you make your class a singleton you have to make sure that your operations are thread-safe.
    • Reading and writing the same file from different threads are handled in a proper way.
  • As pointed out by others you have to do some clean-up as well.
    • To make sure that if something goes wrong then your file handles are closed properly.

Performance

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.