Persist data by serializing/deserializing objects that are sent to it

Question

I've created the following class to persist data by serializing/deserializing objects that are sent to it. I would like to know if there is a better way of writing this class, or if my class is fine the way it is.

using System.IO;
using System.Runtime.Serialization.Formatters.Binary;
using System.Runtime.Serialization.Formatters.Soap;

namespace Education_PersistingData
{
public class PersistData<T>
{
    private readonly T _obj;
    private readonly string _filePath;
    private readonly string _fileName;

    public PersistData(T obj, string filePath, string fileName)
    {
        this._obj = obj;
        this._filePath = filePath;
        this._fileName = fileName;
    }

    /// <summary>
    /// Serializes objects to a SOAP .xml format
    /// </summary>
    public void SerializeToSoapFormat()
    {
        try
        {
            SoapFormatter soapFormatter = new SoapFormatter();
            Stream dataStream = File.Create(_filePath + _fileName);
            soapFormatter.Serialize(dataStream, _obj);
            dataStream.Close();
        }
        catch (IOException ex)
        {
            throw new IOException(ex.Message);
        }
    }

    /// <summary>
    /// Serializes objects to a Binary .txt format
    /// </summary>
    public void SerializeToBinaryFormat()
    {
        try
        {
            BinaryFormatter binaryFormatter = new BinaryFormatter();
            Stream dataStream = File.Create(_filePath + _fileName);
            binaryFormatter.Serialize(dataStream, _obj);
            dataStream.Close();
        }
        catch (IOException ex)
        {
            throw new IOException(ex.Message);
        }
    }

    /// <summary>
    /// Deserializes a SOAP .xml file format
    /// </summary>
    /// <returns>Deserialized object</returns>
    public T DeserializeSoapFormat()
    {
        try
        {
            SoapFormatter soapFormatter = new SoapFormatter();
            Stream dataStream = File.OpenRead(_filePath + _fileName);
            T result = (T)soapFormatter.Deserialize(dataStream);
            dataStream.Close();
            return result;
        }
        catch (IOException ex)
        {
            throw new IOException(ex.Message);
        }
    }

    /// <summary>
    /// Deserializes a Binary .txt file format
    /// </summary>
    /// <returns>Deserialized object</returns>
    public T DeserializeBinaryFormat()
    {
        try
        {
            BinaryFormatter binaryFormatter = new BinaryFormatter();
            Stream dataStream = File.OpenRead(_filePath + _fileName);
            T result = (T)binaryFormatter.Deserialize(dataStream);
            dataStream.Close();
            return result;
        }
        catch (IOException ex)
        {
            throw new IOException(ex.Message);
        }
    }
}
}

Answer 1

There is no point in catching the IOException and then throwing a new exception. You only loose information here. Either handle it or remove the try-catch altogether.

Also, use using to always dispose IDisposable objects.

Answer 2

You could try making the class generic, with the restriction that it must implement IFormatter, as both SoapFormatter and BinaryFormatter does.

public class PersistData<T, TY> where TY : IFormatter, new()
{
    private readonly T _obj;
    private readonly string _filePath;
    private readonly string _fileName;

    public PersistData(T obj, string filePath, string fileName)
    {
        this._obj = obj;
        this._filePath = filePath;
        this._fileName = fileName;
    }


    public void Serialize()
    {
        try
        {
            IFormatter formatter = new TY();

            using (Stream dataStream = File.Create(_filePath + _fileName))
            {
                formatter.Serialize(dataStream, _obj);
            }
        }
        catch (IOException ex)
        {
            // log and handle error
        }
    }



    public T Deserialize()
    {
        try
        {
            IFormatter formatter = new TY();

            using (Stream dataStream = File.OpenRead(_filePath + _fileName))
            {
                return (T) formatter.Deserialize(dataStream);
            }
        }
        catch (IOException ex)
        {
             // log and handle error
        }
    }


}

Answer 3

From the looks of the way you've used the class, I would say there is little point having filePath and fileName separate. Instead, I would just pass in the full path. That way, you have all the information you need without having to combine the two strings each time you use them (which if used a large amount could slightly affect performance). If you find you need them separate in some cases, you can always retrieve them using the helper methods in System.IO.Path.

Also, since the formatters are generically used, I would re-use them. So instead of creating a new formatter each time you wish to serialize/deserialize some data, I would put the following (or something similar) at the top of your class:

private static readonly SoapFormatter SharedSoapFormatter = new SoapFormatter();
private static readonly BinaryFormatter SharedBinaryFormatter = new BinaryFormatter();

One final point I would make, is to use a using statement when dealing with streams. This is much nicer and makes the stream disposal more explicit. i.e.

using (var stream = File.Create(filePath))
{
    // Use the stream
} // <- This will call the stream's `Dispose` method.

Other than that (and the wrapping of IOException which has already been mentioned), I would say it appears to be fine.

Answer 4

I would combine some of the suggestions already given and make the class stateless. Storing the object and path in fields does not really buy you anything, and it ensures that you have to construct a new object whenever you want to serialize/deserialize something else.

In addition, I would probably change the name. PersistData is not a good class name (classes should be nouns). I am naming impaired, though, so I leave that as an exercise for the OP :)

public static class PersistData
{
    public static void Serialize<TFormatter>(object obj, string path) where TFormatter : IFormatter, new()
    {
        try
        {
            var formatter = new TFormatter();

            using (Stream dataStream = File.Create(path))
            {
                formatter.Serialize(dataStream, obj);
            }
        }
        catch (IOException ex)
        {
            // log and handle error
        }
    }

    public static T Deserialize<T,TFormatter>(string path) where TFormatter : IFormatter, new()
    {
        try
        {
            var formatter = new TFormatter();

            using (Stream dataStream = File.OpenRead(path))
            {
                return (T) formatter.Deserialize(dataStream);
            }
        }
        catch (IOException ex)
        {
             // log and handle error
        }
    }
}