NewtonSoft Json.Net serialiser

Question

I'm just starting to develop more in C# after being mainly a VB.NET developer and was looking for someone to critique my implementation of a NewtonSoft Json.Net serialiser.

Can you provide some feedback on the following points:

1. Is this a good way to build the class (using Unity)?
2. Is it acceptable to be throwing an exception from the constructor?
3. Is the Async/Await implementation correct?

Interface

using System.Threading.Tasks;
namespace Helper.Core.Serialisation
{
 public interface ISerialiser
 {
 /// <summary>
 /// Serialise the passed in object with the Json.Net serialiser
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A serialised Json string</returns>
 Task<string> SerialiseAsync<T>(T serialseObject);
 /// <summary>
 /// Serialise the passed in object with the Json.Net serialiser and compress the string using the IStreamCompression implementation
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A compressed byte array of the serialised object</returns>
 Task<byte[]> SerailseAndCompressAsync<T>(T serialseObject);
 /// <summary>
 /// Deserialise the Json string into the generic object
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A deserialsied object of type T</returns>
 Task<T> DeserialiseAsync<T>(string serialsedString);
 /// <summary>
 /// Uncompress and deserialise the Json string into the generic object
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialed">The object to be serialised</param>
 /// <returns>An uncompressed & deserialsied object of type T</returns>
 Task<T> DeserialseAndUnCompressAsync<T>(byte[] serialed);
 }
}

Implementation

using System;
using System.Threading.Tasks;
using Helper.Core.Compression;
using Helper.Core.Logging;
using Microsoft.Practices.Unity;
using Newtonsoft.Json;
namespace Helper.Core.Serialisation
{
 /// <summary>
 /// Json.Net implementaiton of the ISerialiser interface
 /// </summary>
 internal class JsonSerialiser : ISerialiser
 {
 private readonly IStreamCompression _streamCompressor;
 private readonly ILogger _logger;
 /// <summary>
 /// Creates a new instance of the Json.Net Serialiser implementaton
 /// </summary>
 /// <param name="streamCompressor">IStreamCompression implementation composed via the IOC container</param>
 /// <param name="logger">ILogger implementation composed via the IOC container</param> 
 [InjectionConstructor]
 public JsonSerialiser(IStreamCompression streamCompressor, ILogger logger)
 {
 if (streamCompressor == null) throw new ArgumentNullException("streamCompressor");
 if (logger == null) throw new ArgumentNullException("logger");
 this._streamCompressor = streamCompressor;
 this._logger = logger;
 }
 /// <summary>
 /// Serialise the passed in object with the Json.Net serialiser
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A serialised Json string</returns>
 public async Task<string> SerialiseAsync<T>(T serialseObject)
 {
 if (serialseObject == null) throw new ArgumentNullException("serialseObject");
 try
 {
 return await JsonConvert.SerializeObjectAsync(serialseObject);
 }
 catch (JsonSerializationException ex)
 {
 _logger.LogEntry(ex);
 throw new SerialisationException("Could Not Serialse The Object", ex);
 }
 }
 /// <summary>
 /// Serialise the passed in object with the Json.Net serialiser and compress the string using the IStreamCompression implementation
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A compressed byte array of the serialised object</returns>
 public async Task<byte[]> SerailseAndCompressAsync<T>(T serialseObject)
 {
 if (serialseObject == null) throw new ArgumentNullException("serialseObject");
 try
 {
 string serialised = await SerialiseAsync(serialseObject);
 return await _streamCompressor.CompressStringAsync(serialised);
 }
 catch (StreamCompressionException ex)
 {
 _logger.LogEntry(ex);
 throw new SerialisationException("Could Not Compress The Object", ex);
 }
 }
 /// <summary>
 /// Deserialise the Json string into the generic object
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialseObject">The object to be serialised</param>
 /// <returns>A deserialsied object of type T</returns>
 public async Task<T> DeserialiseAsync<T>(string serialsedString)
 {
 if (serialsedString == null) throw new ArgumentNullException("serialsedString");
 try
 {
 return await JsonConvert.DeserializeObjectAsync<T>(serialsedString);
 }
 catch (JsonSerializationException ex)
 {
 _logger.LogEntry(ex);
 throw new SerialisationException("Could Not Deserialse The Object", ex);
 }
 }
 /// <summary>
 /// Uncompress and deserialise the Json string into the generic object
 /// </summary>
 /// <typeparam name="T">Generic type of the serialised object</typeparam>
 /// <param name="serialed">The object to be serialised</param>
 /// <returns>An uncompressed & deserialsied object of type T</returns>
 public async Task<T> DeserialseAndUnCompressAsync<T>(byte[] serialed)
 {
 if (serialed == null) throw new ArgumentNullException("serialed");
 try
 {
 string decompressedSerialised = await _streamCompressor.DecompressStringAsync(serialed);
 return await DeserialiseAsync<T>(decompressedSerialised);
 }
 catch (StreamCompressionException ex)
 {
 _logger.LogEntry(ex);
 throw new SerialisationException("Could Not Decompress The Object", ex);
 }
 }
 }
}

• dreza: I don't see anything wrong with throwing the exception in the constructor. If the parameter is required, it's required. At least caught in the constructor you have a chance to review the call stack and find out why
• Paritosh: I agree with dreza , constructing an object should be avoided if some of necessary parameter are not present during constructing an object.
• Danil: As all methods in interface has async postfix, maybe rename ISerializer to IAsyncSerializer and remove async postfix in methods names.
• Ewan: surely it would be better to allow a null stringcompressor/logger rather than throw an exception in a constructor. It seems simple and logical that null wont log or compress?

Answer 1

Code looks good, I like this IoC style. 3 points to your consideration:

1. You should catch an AggregateException over await.
2. I wouldn't bother passing a logger to a serializer - that's none of his business. Let the serializer throw if he's not happy.
3. Fix some typo in names and messages ("Deserialse" and so).
4. I somewhat doubt the whole concept of async serialization. I take serialized data to be an object snapshot in a known 'time point'. But if it's useful for you go for it.

(Oops didn't address your actual questions)

1. Yes I think it's great.
2. Sure. Lacking meaningful 'default object', you don't have many alternatives.
3. This is probably the main issue here, and the hardest to answer. I have some doubts about returning a non-cancellable Task. I suspect if the object to be serialized has changed completely, the user may want to cancel the serialization.

Answer 2

A minor point, but what if you want to create a single threaded Serializer as well?

I would rename your interface to something like IAsyncSerializer so that you could have a different interface ISerializer.