Tuesday, June 2, 2009

Why You Should Never Make Your Business Objects DataContracts

Once upon a time, there was a fledgling young developer. He wrote a nice, clean business object.


public class NiceCleanBusinessObject
{
public string Value { get; set; }

public string LastUpdatedBy { get; set; }

public DateTime? LastUpdatedOn { get; set; }
}


He liked to share, so one day, he decided to use WCF so his business object can be shared with other people. Not seeing a point in having a separate DataContract class from his business object, and wanting to save some time, he decorated his business objects with DataContract and DataMember attributes so he could avoid writing a translator class, which he considered an extra level of abstraction. His nice, clean business object then looked like this:


[DataContract]
public class NiceCleanBusinessObject
{
[DataMember]
public string Value { get; set; }

[DataMember]
public string LastUpdatedBy { get; set; }

[DataMember]
public DateTime? LastUpdatedOn { get; set; }
}


"That was easy!", he thought to himself, and went home for the afternoon.

The next day, his employer asked him to be sure to clear the LastUpdatedBy and LastUpdatedDate whenever the Value property was set, so he changed his business object again:


[DataContract]
public class NiceCleanBusinessObject
{
private string _value;

[DataMember]
public string Value
{
get { return _value; }
set
{
_value = value;
LastUpdatedBy = null;
LastUpdatedOn = null;
}
}

[DataMember]
public string LastUpdatedBy { get; set; }

[DataMember]
public DateTime? LastUpdatedOn { get; set; }
}


He sighed a happy sigh and went home.

The next day, his boss expresses interest in creating a holder class to transport several nice clean business objects across the wire. These business objects should be on a list in another object. Our hero creates this class:


[DataContract]
[KnownType(typeof(NiceCleanBusinessObject))]
public class NiceCleanParentObject
{
public NiceCleanParentObject()
{
NiceCleanObjects = new List<NiceCleanBusinessObject>();
}

[DataMember]
public List<NiceCleanBusinessObject> NiceCleanObjects { get; set; }
}


He's happy everything works, so he goes home.

The next day, he gets a request to create a new property, Value2. Value2 is set whenever Value is set, but can also be overridden by the user after Value is changed. But whenever Value is changed now, information from the NiceCleanParentObject must be used to set the information on Value2. So his code now looks like this:


[DataContract]
[KnownType(typeof(NiceCleanBusinessObject))]
public class NiceCleanParentObject
{
public NiceCleanParentObject()
{
NiceCleanObjects = new List<NiceCleanBusinessObject>();
}

[DataMember]
public string Value { get; set; }

[DataMember]
public List<NiceCleanBusinessObject> NiceCleanObjects { get; set; }
}

[DataContract]
public class NiceCleanBusinessObject
{
private string _value;

[DataMember]
public string Value
{
get { return _value; }
set
{
_value = value;
LastUpdatedBy = null;
LastUpdatedOn = null;
RunSomethingComplicated();
}
}

[DataMember]
public string Value2 { get; set; }

[DataMember]
public string LastUpdatedBy { get; set; }

[DataMember]
public DateTime? LastUpdatedOn { get; set; }

public NiceCleanParentObject Parent { get; set; }

private void RunSomethingComplicated()
{
Value2 = Parent.Value;
}
}


He compiles his code, and it won't run properly. Oh no! What shall he do? Every time he tries to pass the object over WCF, he gets a NullReferenceException. In hunting it down (which takes significantly longer than it should), it turns out that while serializing and deserializing a property, the business logic associated with the property actually gets executed. This is because setters and getters are actually called whenever the DataContractSerializer is running, and it's starting to affect his business object's data. He has a couple of options at this point:

1. He can serialize just the fields and automatic properties. He doesn't particularly like this idea, because other people are relying on the DataContract at this point, and relying on it to be in a particular format. This could become a little sticky for them.
2. He can create a base class that carries a flag that determines the current state of the object, whether it's being serialized or not. He decides this is a good idea.
3. He could try to serialize the parent as well. This has the advantage of giving access to the parent properties, but it's still relying on the implemented logic of the DataContractSerializer, in hoping that the Parent object's properties that are requested to be used are serialized before the List of NiceCleanBusinessObjects. Also, serializing the Parent property, when it carries a property containing a list of children creates cycles in the serialization process, because the parent serializes the children, which serializes the parent and so on so forth, so a workaround has to be made. Nevertheless, his UI architecture is depending on the fact that this Business Object self-updates itself whenever a property is set, so he still needs a reference to the parent. Our hero finds his workaround here.

So he implements numbers 2 and 3 above, and comes up with this:


[DataContract]
[KnownType(typeof(NiceCleanBusinessObject))]
public class NiceCleanParentObject
{
public NiceCleanParentObject()
{
NiceCleanObjects = new List<NiceCleanBusinessObject>();
}

[DataMember]
public string Value { get; set; }

[DataMember]
public List<NiceCleanBusinessObject> NiceCleanObjects { get; set; }
}

[DataContract, KnownType(typeof(NiceCleanBusinessObject))]
public class NiceCleanBusinessObjectBase
{
[DataMember]
protected bool _isSerializing;

[OnSerializing]
protected void OnSerializing(StreamingContext context)
{
_isSerializing = true;
}

[OnSerialized]
protected void OnSerialized(StreamingContext context)
{
_isSerializing = false;
}

[OnDeserializing]
protected void OnDeserializing(StreamingContext context)
{
_isSerializing = true;
}

[OnDeserialized]
protected void OnDeserialized(StreamingContext context)
{
_isSerializing = false;
}
}

[DataContract]
public class NiceCleanBusinessObject : NiceCleanBusinessObjectBase
{
private string _value;

[DataMember]
public string Value
{
get { return _value; }
set
{
_value = value;

if (!_isSerializing)
{
LastUpdatedBy = null;
LastUpdatedOn = null;
RunSomethingComplicated();
}
}
}

[DataMember]
public string Value2 { get; set; }

[DataMember]
public string LastUpdatedBy { get; set; }

[DataMember]
public DateTime? LastUpdatedOn { get; set; }

[DataMember]
public NiceCleanParentObject Parent { get; set; }

private void RunSomethingComplicated()
{
Value2 = Parent.Value;
}
}


Oh wow. This isn't so much of a nice clean business object anymore, now is it?

Well, I wish I could post more, but our time is short, so I'll tell you what ultimately happened. The business logic inside the business objects became so complicated, due to the fact that the data contract is the business object itself, that our hero ultimately decides that it would be better to switch to an anemic domain model, blasted by such architecture greats as Martin Fowler and several others.

Ultimately, he, and his team members have to support this model that is the result of his bad decision early on in the project, and he's had to learn a difficult lesson:

Don't make your business objects into data contracts. The translation layer is less work in the long run.