views:

370

answers:

2

My business objects are coded with the following architecture:

  • validation of any incoming data throws an exception in the setter if it doesn't fit business logic.
    • property can not be corrupt/inconsistent state unless the existing default/null is invalid
  • business objects can only be created by the business module via a static factory type method that accepts an interface implementation that is shared with the business object for copying into the business object.
    • Enforces that the dependency container, ui, and persistence layers can not create an invalid Model object or pass it anywhere.
  • This factory method catches all the different validation exceptions in a validation dictionary so that when the validation attempts are complete, the dictionary the caller provided is filled with field names and messages, and an exception is thrown if any of the validations did not pass.
    • easily maps back to UI fields with appropriate error messages
  • No database/persistence type methods are on the business objects
  • needed persistence behaviors are defined via repository interfaces in the business module

Sample Business object interface:

public interface IAmARegistration
{
  string Nbk { get; set; } //Primary key?
  string Name { get; set; }
  string Email { get; set; }
  string MailCode { get; set; }
  string TelephoneNumber { get; set; }
  int? OrganizationId { get; set; }
  int? OrganizationSponsorId { get; set; }
 }

business object repository interface:

 /// <summary>
 /// Handles registration persistance or an in-memory repository for testing
 /// requires a business object instead of interface type to enforce validation
 /// </summary>
 public interface IAmARegistrationRepository
 {
  /// <summary>
  /// Checks if a registration record exists in the persistance mechanism
  /// </summary>
  /// <param name="user">Takes a bare NBK</param>
  /// <returns></returns>
   bool IsRegistered(string user); //Cache the result if so

  /// <summary>
  /// Returns null if none exist
  /// </summary>
  /// <param name="user">Takes a bare NBK</param>
  /// <returns></returns>
   IAmARegistration GetRegistration(string user);

   void EditRegistration(string user,ModelRegistration registration);

   void CreateRegistration(ModelRegistration registration);
  }

Then an actual business object looks as follows:

public class ModelRegistration : IAmARegistration//,IDataErrorInfo
{
    internal ModelRegistration() { }
    public string Nbk
    {
        get
        {
            return _nbk;
        }
        set
        {
            if (String.IsNullOrEmpty(value))
                throw new ArgumentException("Nbk is required");
            _nbk = value;
        }
    }
    ... //other properties omitted
    public static ModelRegistration CreateModelAssessment(IValidationDictionary validation, IAmARegistration source)
    {

        var result = CopyData(() => new ModelRegistration(), source, false, null);
        //Any other complex validation goes here
        return result;
    }

    /// <summary>
    /// This is validated in a unit test to ensure accuracy and that it is not out of sync with 
    /// the number of members the interface has
    /// </summary>
    public static Dictionary<string, Action> GenerateActionDictionary<T>(T dest, IAmARegistration source, bool includeIdentifier)
where T : IAmARegistration
    {
        var result = new Dictionary<string, Action>
            {
                {Member.Name<IAmARegistration>(x=>x.Email),
                    ()=>dest.Email=source.Email},
                {Member.Name<IAmARegistration>(x=>x.MailCode),
                    ()=>dest.MailCode=source.MailCode},
                {Member.Name<IAmARegistration>(x=>x.Name),
                    ()=>dest.Name=source.Name},
                {Member.Name<IAmARegistration>(x=>x.Nbk),
                    ()=>dest.Nbk=source.Nbk},
                {Member.Name<IAmARegistration>(x=>x.OrganizationId),
                    ()=>dest.OrganizationId=source.OrganizationId},
                {Member.Name<IAmARegistration>(x=>x.OrganizationSponsorId),
                    ()=>dest.OrganizationSponsorId=source.OrganizationSponsorId},
                {Member.Name<IAmARegistration>(x=>x.TelephoneNumber),
                    ()=>dest.TelephoneNumber=source.TelephoneNumber}, 

            };

        return result;

    }
    /// <summary>
    /// Designed for copying the model to the db persistence object or ui display object
    /// </summary>
    public static T CopyData<T>(Func<T> creator, IAmARegistration source, bool includeIdentifier,
        ICollection<string> excludeList) where T : IAmARegistration
    {
        return CopyDictionary<T, IAmARegistration>.CopyData(
            GenerateActionDictionary, creator, source, includeIdentifier, excludeList);
    }

    /// <summary>
    /// Designed for copying the ui to the model 
    /// </summary>
    public static T CopyData<T>(IValidationDictionary validation, Func<T> creator,
        IAmARegistration source, bool includeIdentifier, ICollection<string> excludeList)
         where T : IAmARegistration
    {
        return CopyDictionary<T, IAmARegistration>.CopyData(
            GenerateActionDictionary, validation, creator, source, includeIdentifier, excludeList);
    }

Sample repository method that I'm having trouble writing isolated tests for:

    public void CreateRegistration(ModelRegistration registration)
    {
        var dbRegistration = ModelRegistration.CopyData(()=>new Registration(), registration, false, null);

       using (var dc=new LQDev202DataContext())
       {
           dc.Registrations.InsertOnSubmit(dbRegistration);
           dc.SubmitChanges();
       }
    }

Issues:

  • When a new member is added there are a minimum of 8 places a change must be made (db, linq-to-sql designer, model Interface, model property, model copy dictionary, ui, ui DTO, unit test
  • Testability
    • testing the db methods that are hard coded to depend on an exact type that has no public default constructor, and needs to pass through another method, makes testing in isolation either impossible, or will need to intrude on the business object to make it more testable.
    • Using InternalsVisibleTo so that the BusinessModel.Tests has access to the internal contructor, but I would need to add that for any other persistence layer testing module, making it scale very poorly
  • to make the copy functionality generic the business objects were required to have public setters
    • I'd prefer if the model objects were immutable
  • DTOs are required for the UI to attempt any data validation

I'm shooting for complete reusability of this business layer with other persistence mechanisms and ui mechanisms (windows forms, asp.net, asp.net mvc 1, etc...). Also that team members can develop against this business layer/architecture with a minimum of difficulty.

Is there a way to enforce immutable validated model objects, or enforce that neither the ui or persistance layer can get a hold of an invalid one without these headaches?

+1  A: 

This looks very complicated to me.

IMO, Domain Objects should be POCOs that protect their invariants. You don't need a factory to do that: simply demand any necessary values in the constructor and provide valid defaults for the rest.

Whereever you have property setters, protect them by calling a validation method like you already do. In short, you must never allow an instance to be in an incosistent state - factory or no factory.

Writing an immutable object in C# is easy - just make sure that all fields are declared with the readonly keyword.

However, be aware that if you follow Domain-Driven Design, Domain objects tend to fall in three buckets

  • Entities. Mutable objects with long-lived identities
  • Value Objects. Immutable objects without identity
  • Services. Stateless

According to this defintion, only Value Objects should be immutable.

Mark Seemann
I was hoping since being a valid business object is a concern of the business, I could enforce the validation in that layer in a clean consistent way.
Maslow
You can, and you should. However, protecting invariants and validating data are two slightly different things. In the first case, you want to make it *impossible* to assign invalid data to a Domain Object. In the second case, you want to be able to ask whether a given input is valid or not. You can have both by encapsulating the validation logic in a separate class, while still have the Domian Object protecting its invariants using this encapsulated validation logic. It has little to do with immmutability.
Mark Seemann
I'm trying to follow, I hope this isn't what you've already answered. It remains that either the caller would have to check the domain object to make sure it is valid before passing it off, those that the object is passed to would have to check it was valid before utilizing it, or the repository methods would need to live in the domain object that call the actual persistence layer?
Maslow
No consumers should ever have to check that a Domain Object in itself is valid because it should be impossible for it to ever get into an invalid state. Validation is mostly useful when you want to map ViewModels to Domain Objects because you want to be able to ask a Validator whether the combined values contained in the ViewModel can be used to construct a valid Domain Object - without relying on try/catch, that is.
Mark Seemann
so then a domain object would require any arguments in the constructor needed to make it valid or require all arguments it would make use of? (this could be a 10 argument constructor, that's ok?). properties would not have setters, or the setters would throw exceptions if the value placed them in an invalid state? Validation logic for a business object would live in a separate class from the business object, but would validate the model, and view model because they would implement the same interface which would provide properties with get accessors.
Maslow
@Mark are you saying saying A) "validation" applies at construction time, and B) since constructor input comes from the view, it makes sense to validate view models separately of domain object construction, and therefore validation logic should live separate from the domain object?
gWiz
@Maslow, regarding properties on domain objects that are immutable after construction, I would make the getters public and the setters private or protected. Although I'm not sure if this plays nice with things like NHibernate, which makes some demands on domain object design.
gWiz
@gWiz Mark seems to be saying immutable domain objects are not wise or helpful. which leads me to the possibility of an immutable DTO living in the business layer for passing to the repository perhaps.
Maslow
This discussion is becoming almost longer than both the question and my original answer, so I'd suggest that you open some new dedicated questions where we can discuss better than within these limited comments.
Mark Seemann
However, two points I'd like to make is: 1) No, I don't think 10-argument constructors are OK, but that's why we have the Parameter Object refactoring/pattern. 2) I never said that immutable entities are a bad idea - I just said (over at the Manning author forum, IIRC) that I haven't seen this done before. I'm still thinking this one over, but I'd be tempted to try it out on a small project :)
Mark Seemann
A: 

I have followed a different approach in the past. Rather than protecting property setters with validation exceptions, I adopt the convention that A) all domain objects provide a way to validate themselves on-demand (e.g. a Validate method), and B) repositories assert the validation of all objects on which persistence operations are requested (e.g. by invoking the Validate method and throwing an exception if it fails). This means that to use a repository is to trust that the repository upholds this convention.

The advantage of this, in the .NET world at least, is an easier-to-use domain surface and more succinct and readable code (no huge try-catch blocks or roundabout exception handling mechanisms). This also integrates well with ASP.NET MVC validation approach.

I will say that in non-property operations exposed by domain objects (such as those implied by the Law of Demeter over composed collections), I've tended to return a ValidationResult that provides immediate validation feedback. This feedback is also returned by the Validate methods. The reason I do this is that there is no detrimental impact to the structure or readability of consuming code. If the caller doesn't want to inspect the return value, they don't have to; if they do want to, they can. Such a benefit is not possible with property setters without also impacting the structure of the consuming code (with try-catch blocks). Likewise, domain services return ValidationResults. Only repositories throw ValidationExceptions.

Overall, this does allow you to put domain objects into invalid states, but they are isolated to the caller's execution context (the caller's "workspace") and cannot be persisted to the system at large.

gWiz
it just seems more natural that validation of business logic live in the business layer.
Maslow
What do you mean? The approach I describe keeps the implementation of validation logic in the business layer. This supports encapsulation and cohesion, as well as the Common Closure Principle, all of which are "why" it seems more natural for validation to live with the business logic. When and how validation is performed is a matter of convention, and not an inherent object-oriented principle.
gWiz
I mean that the repository/persistence layer in your example is responsible for remembering to check if the business logic is appropriate, instead of the business layer.
Maslow
Yes, that's why I said "This means that to use a repository is to trust that the repository upholds this convention." In your sample code, the burden is on the consumer/application to put a try-catch around every property setter. IMHO, I feel it's important to keep the application code as clean as possible, since it has to deal with so many other things (auth, session, UI), and my technique promotes cleanliness. The app can explicitly call Validate before trying to save, and not include *any* try-catch blocks at all.
gWiz
Regarding trusting the repository to uphold the convention, any time you have dependency injection, you're putting the responsibility for choosing the dependencies into the hands of the application. One way or another, the app must verify the trust of each dependency. The is true regardless of the validation convention in place. In my case, my domain documentation explains the validation convention and the responsibilities of the repository. It is up to the app to verify that the repositories it uses uphold the convention.
gWiz
perhaps my sample code didn't show enough of the usage. the ui/presentation layers are expected to have their own DTO that implements the interface. When data is submitted, it would then go through the validation process where the try/catch setup lives, which is in the business layer. So no try/catch messiness is needed for the consumer. If I removed the property setting exception throws in favor of a validation method, only one try catch would be needed, in the consumer when it tried to validate data and failed.
Maslow
as far as trusting vs dependency injection, having the business layer internally handle its validation in a self-contained, strict sense I don't have to involve dependency injection in that layer besides what is needed to allow for testability.
Maslow