1

So, I came upon the code similar to the example below. One thing that bothered me about it was the fact that it's dependent on a Result object being created in the Application Service then passed down into the Domain Entities. This struck me as odd, as it's exposing parts of the domain. It's also receiving a parameter that shouldn't ever have any value other than the default - an empty/default/success Result.

I suggested to make the public methods return Result instead (either instead of void or as an out parameter), while keeping the internal methods the same. The argument against this is that it's more difficult to include all errors in the same result if you're not passing the result down into Save, for example..

It's worth noting that there are two aggregate roots here, and they overlap. When editing Parents or Children, both Parent and Child have an aggregate root of Parent. When copying, all three classes have an aggregate root of Grandparent.

What's the best way to proceed, here? Optional parameters? Redefine the aggregates so everything goes through Grandparent? Have the lower Copy methods return a List of Result that can be collapsed into a single one in Grandparent? Is there something we're missing?

    public class Grandparent
    {
        private IEnumerable<Parent> Parents;
        private int Id;

        public void Save(Result result)
        {
            Validate(result);
            /*Save*/
        }

        public void Copy(Result result)
        {
            Id = 0;
            Save(result);
            foreach (var parent in Parents) parent.Copy(result, Id);
        }

        private void Validate(Result result) { /*Update result if there are any errors*/ }
    }
    public class Parent
    {
        private IEnumerable<Child> Children;
        private int Id;
        private int GrandparentId;

        public void Save(Result result)
        {
            Validate(result);
            /*Save*/
        }

        public void SaveChildren(Result result, ICollection<Child> dto)
        {
            /*Do stuff with dto*/
            foreach (var child in Children) child.Save(result);
        }

        private void Validate(Result result) { /*Update result if there are any errors*/ }

        public void Copy(Result result, int grandparentId)
        {
            Id = 0;
            GrandparentId = grandparentId;
            Save(result);
            foreach (var child in Children) child.Copy(result, Id);
        }
    }

    public class Child
    {
        private int Id;
        private int ParentId;
        internal void Save(Result result)
        {
            Validate(result);
            /*Do thing*/
        }

        private void Validate(Result result) { /*Update result if there are any errors*/ }

        public void Copy(Result result, int parentId)
        {
            Id = 0;
            ParentId = parentId;
            Save(result);
        }
    }
  • 3
    What is your specific goal? – Robert Harvey Oct 23 '19 at 18:42
  • @RobertHarvey Clean/maintainable code. – Sarov Oct 23 '19 at 18:43
  • I suggest, pulling out your logic out of your entity/model classes. Create service and data layer write your business logic and DB transactions there. Follow Single Responsibility and Separation of concerns. – Mustafa Mohammadi Oct 23 '19 at 20:08
  • @MustafaMohammadi So an anemic domain model? – Sarov Oct 23 '19 at 20:13
  • @Sarov I would suggest Multi-tier/multi layer architecture en.wikipedia.org/wiki/Multitier_architecture – Mustafa Mohammadi Oct 23 '19 at 20:22
1

In situations like this I try to think about whoever is going to have to write the code that uses it. In your organization's current design, it might look like this, yes?

var result = new Result();
obj.Validate(result);
obj.Save(result);
LogResult(result);

I think what you are suggesting might look like this (which is fine with me):

var results = new Results();
results.Add(obj.Validate());
results.Add(obj.Save());
LogResults(results);

Or (same thing with initializer syntax):

var results = new Results
{
    obj.Validate(),
    obj.Save()
};
LogResults(results);

...which isn't so bad.

But if you want to go an extra mile, use lambdas:

var results = new Transaction(obj)
{
    o => o.Validate(),
    o => o.Save()
}.ToResults();
LogResults(results);

This approach gives you a lot more control and extensible. It would be fairly simple, for example, to modify this for parallel or asynchronous execution, or introduce performance counters or preamble/postamble logging.

If you really want to go nuts, add an extension method that takes a logging delegate. Then you do everything in one line. You don't even need a variable! I would probably never actually write it that way, but this demonstrates how the interface simplifies things for the caller.

new Transaction(obj)
{
    o => o.Validate(),
    o => o.Save()
}
.ToResults()
.Log
( 
    r => LogResults(r) 
);
| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.