views:

79

answers:

1

I was looking for some advice on designing a data model for contract administration. The general life cycle of a contract is thus:

  • Contract is created and in a "draft" state. It is viewable internally and changes may be made.
  • Contract goes out to vendor, status is set to "pending"
  • Contract is rejected by vendor. At this state, nothing can be done to the contract. No statuses may be added to the collection.
  • Contract is accepted by vendor. At this state, nothing can be done to the contract. No statuses may be added to the collection.

I obviously want to avoid a situation where the contract is accepted and, say, the amount is changed. Here are my classes:

[EnforceNoChangesAfterDraftState]
public class VendorContract 
{
 public virtual Vendor Vendor { get; set; }            
 public virtual decimal Amount { get; set; }
 public virtual VendorContact VendorContact { get; set; }              
 public virtual string CreatedBy { get; set; }
 public virtual DateTime CreatedOn { get; set; }
 public virtual FileStore Contract { get; set; }

 public virtual IList<VendorContractStatus> ContractStatus { get; set; } 
}        

[EnforceCorrectWorkflow]
public class VendorContractStatus
{
  public virtual VendorContract VendorContract { get; set; }
  public virtual FileStore ExecutedDocument { get; set; }
  public virtual string Status { get; set; }
  public virtual string Reason { get; set; }
  public virtual string CreatedBy { get; set; }
  public virtual DateTime CreatedOn { get; set; }
}

I've omitted the filestore class, which is basically a key/value lookup to find the document based on its guid.

The VendorContractStatus is mapped as a many-to-one in Nhibernate.

I then use a custom validator as described here. If anything but draft is returned in the VendorContractStatus collection, no changes are allowed. Furthermore the VendorContractStatus must follow the correct workflow (you can add a rejected after a pending, but you can't add anything else to the collection if a reject or accepted exists, etc.).

All sounds alright? Well a colleague has argued that we should simply add an "IsDraft" bool property to VendorContract and not accept updates if IsDraft is false. Then we should setup a method inside of VendorContractStatus for updating the status, if something gets added after a draft, it sets the IsDraft property of VendorContract to false.

I do not like this as it feels like I'm dirtying up the POCOs and adding logic that should persist in the validation area, that no rules should really exist in these classes and they shouldn't be aware of their states.

Any thoughts on this and what is the better practice from a DDD perspective?

From my view, if in the future we want more complex rules, my way will be more maintainable over the long run. Say we have contracts over a certain amount to be approved by a manager. I would think it would be better to have a one-to-one mapping with a VendorContractApproval class, rather than adding IsApproved properties, but that's just speculation.

This might be splitting hairs, but this is the first real gritty enterprise software project we've done. Any advice would be appreciated!

A: 

The fact that rules aren't enforced until callers try to persist a VendorContract instance probably makes bugs more likely, and makes those classes harder to use because they don't describe their own behavior or constraints.

To put this another way by focusing on a single feature: rejected and accepted contracts shouldn't even have a set accessor for Amount.

It sounds like you may be heading toward an anemic domain model (sometimes known by the less prejudicial name 'persistent model') where your domain objects are just containers with no business logic. That's not always a bad thing, but in systems with a lot of business logic that style risks duplicating code and spreading rules out over too many objects, making it extremely difficult to understand (and maintain and change) the rules that govern your domain entities.

Jeff Sternal
Thanks. I agree with this, I think what we've decided to do is break out the classes into VendorContractDraft and VendorContract, since these are the two states they can be in from a validation standpoint. This way we can more naturally markup the properties with validation logic, rather than doing it in a separate validation class.
chum of chance