Spot the Bug - Using Statement Part 2

I introduced the "Spot the Bug" exercise previously. This is the next bug in the series.

Below is a class named DataWriter. This class is responsible for opening a file, closing a file, and writing data to a file. Below that class is a console program that exercises the DataWriter class. Can you spot the bug?

	using System;
	using System.IO;
	using System.Collections.Generic;
	namespace FileHandles
	{
	public class DataWriter
	{
	public StreamWriter OpenFile(string fileName)
	{
	return new StreamWriter(fileName);
	}
	public bool WriteData(StreamWriter writer, List<string> data)
	{
	foreach (var line in data)
	{
	writer.WriteLine(line);
	}
	return true;
	}
	public bool CloseFile(StreamWriter writer)
	{
	writer.Close();
	writer.Dispose();
	return true;
	}
	}
	public class Program
	{
	public static void Main(string[] args)
	{
	var dataWriter = new DataWriter();
	var writer = dataWriter.OpenFile("data.txt");
	var lines = new List<string>
	{
	"This is the first string.",
	"Another string that needs writing.",
	"Yet another string for writing."
	};
	var result = dataWriter.WriteData(writer, lines);
	}
	}
	}

view raw FileHandles_Buggy.cs hosted with ❤ by GitHub

In the console program, I forgot to call the method to close the file. This is another example of the using statement in C# protecting us from creating code like this example. In the previous example, we saw a similar bug where it is very easy for the developer to forget to close a connection or file handle, using up precious resources. The using statement isn't a perfect solution, but it does push the developer to keep responsibilities grouped appropriately and it ensures that the garbage collector can do some amount of cleanup in the system when resources are no longer needed.

Below is code that represents a better solution.

	using System;
	using System.IO;
	using System.Collections.Generic;
	namespace FileHandles
	{
	public class Program
	{
	public static void Main(string[] args)
	{
	var lines = new List<string>
	{
	"This is the first string.",
	"Another string that needs writing.",
	"Yet another string for writing."
	};
	using(var dataWriter = new StreamWriter("data.txt"))
	{
	dataWriter.Open();
	foreach(var line in lines)
	{
	writer.WriteLine(line);
	}
	dataWriter.Close(); // Good practice
	}
	}
	}
	}

view raw FileHandles_Better.cs hosted with ❤ by GitHub

The code above uses the using statement to handle the opening of the file. The developer is still responsible for closing the file, but the using statement will make sure that the StreamWriter object is disposed.

The other thing to notice in the better code example is that the DataWriter class was completely eliminated. Because the DataWriter class split up the file writing responsibilities too granularly, once those responsibilities were merged back together, the class was no longer necessary. This example highlights the fact that sometimes our designs aren't quite right, and we shouldn't be afraid to remove classes if they aren't necessary.