0

I've just run into a problem while trying to re-design our existing business objects to a adopt a more domain-driven design approach.

Currently, I have a Product Return aggregate root, which handles data related to a return for a specific product. As part of this aggregate, a date needs to be provided to say what month (and year) the return is currently for.

Each product return MUST be sequential, and so each product return must be the next month after the previous. Attempting to create a product return that doesn't follow this pattern should result in an exception.

I had thought about passing along a Domain Service to the method (or constructor) that sets the PeriodDate for the return, but I'm at a loss for how I would do this. Even if the domain service had a reference to a repository, I can't see it being appropriate to put a "GetNextReturnDate()" on that repository.

For background, each product return is associated with a product. I was reluctant to make the product the aggregate root, as loading up all the product returns just to add one seemed like an extremely non-performant way of doing things (considering this library is going to be used with a RESTful Web API).

Can anyone provide suggestions as to how I should model this? Is it a matter of just changing the aggregate root and dealing with the performance? Is there some place in the Domain that 'query' type services can be placed?

As an example, the current constructor the for product return looks like this:

public ProductReturn(int productID, int estimateTypeID, IProductService productService)
{
     // This doesn't feel right, and I'm not sure how to implement it...
     _periodDate = productService.GetNextReturnDate(productID);

    // Other initialization code here...
}

The IProductService (and it's implementation) sit in the Domain layer, so there's no ability to call SQL directly from there (and I feel that's not what I should be doing here anyway)

Again, in all likelihood I've modelled this terribly, or I've missed something when designing the aggregate, so any help would be appreciated!

I Think my broader problem here is understanding how to implement constraints (be it foreign, unique, etc.) inside of a domain entity, without fetching an entire list of returns via a domain service, when a simple SQL query would give me the information required

EDIT: I did see this answer to another question: https://stackoverflow.com/a/48202644/9303178, which suggests having 'Domain Query' interfaces in the domain, which sound like they could return the sort of data I'm looking for.

However, I'm still worried I'm missing something here with my design, so again, I'm all open to suggestions.

EDIT 2: In response to VoiceOfUnreason's answer below, I figured I'd clarify a few things RE the PeriodDate property.

The rules on it are as follows:

  1. CANNOT be NULL
  2. It MUST be in sequence with other product returns, and it cannot be in a valid state without this fulfilled

It's a tricky one. I can't rely on the date passed in because it could then very well be out of sequence, but I can't figure out the date without the service being injected. I am going to transform the constructor into a method on a Factory to remove the 'constructor doing work' anti pattern.

I may be overly defensive over the way the code is currently, but it feels the amount of times the ReturnService has to be injected is wrong. Mind you, there are lots of scenarios where the return value must be recalculated, but it feels as though it would be easy to do this just before a save (but I couldn't think of a clean way to do this).

Overall I just feel this class has a bit of a smell to it (with the injected services and whatnot), but I may be needlessly worrying.

Olorin
  • 21
  • 7
  • could you explain little bit more on "Each product return MUST be sequential". What i understood ProductReturn is: many people are buying products from your site , and if somebody wants to return a Product , you are creating a Product Return Agg. , in that case what is the significance of "each product return must be sequential". please clarify – techagrammer Feb 02 '18 at 06:21
  • i think you should ask this question on software engineering forum , you will get good response there as it looks a design problem . Link - https://softwareengineering.stackexchange.com/ – Sahil Aggarwal Feb 02 '18 at 06:28
  • @SahilAggarwal when referring other sites, it is often helpful to point that [cross-posting is frowned upon](https://meta.stackexchange.com/tags/cross-posting/info) – gnat Feb 02 '18 at 08:39
  • @techagrammer it’s in reference to finance - a product is essentially an investment. Being an investment, it has monthly returns. – Olorin Feb 02 '18 at 09:42

2 Answers2

0

I had thought about passing along a Domain Service to the method (or constructor) that sets the PeriodDate for the return, but I'm at a loss for how I would do this.

I strongly suspect that passing a domain service to the method is the right approach to take.

A way of thinking about this: fundamentally, the aggregate root is a bag of cached data, and methods for changing the contents of the bag of data. During any given function call, it's entire knowledge of the world is that bag, plus the arguments that have been passed to the method.

So if you want to tell the aggregate something that it doesn't already know -- data that isn't currently in the bag -- then you have to pass that data as an argument.

That in turn comes in two forms; if you can know without looking in the aggregate bag what data to pass in, you just pass it as an argument. If you need some of the information hidden in the aggregate bag, then you pass a domain service, and let the aggregate (which has access to the contents of the bag), pass in the necessary data.

public ProductReturn(int productID, int estimateTypeID, IProductService productService)
{
    // This doesn't feel right, and I'm not sure how to implement it...
    _periodDate = productService.GetNextReturnDate(productID);
    // Other initialization code here...
}

This spelling is a little bit odd; constructor does work is usually an anti-pattern, and it's a bit weird to pass in a service to compute a value, when you could just compute the value and pass it in.

If you feel like it's part of the business logic to decide how to compute _periodDate, which is to say if you think the rules for choosing the periodDate belong to the ProductReturn, then you would normally use a method on the object to encapsulate those rules. On the other hand, if periodDate is really decided outside of this aggregate (like productID is, in your example), then just pass in the right answer.

One idea that may be crossing you up: time is not something that exists in the aggregate bag. Time is an input; if the business rules need to know the current time to do some work, then you will pass that time to the aggregate as an argument (again, either as data, or as a domain service).

The user can’t pass a date in, because at any given time, the date for a return can ONLY be the next date from the last return.

Typically, you have a layer sitting between the user and the domain model -- the application; it's the application that decides what arguments to pass to the domain model. For instance, it would normally be the application that is passing the "current time" to the domain model.

On the other hand, if the "date of the last return" is something owned by the domain model, then it probably makes more sense to pass a domain service along.

I should also mention - a return is invalid without a date, so I can’t construct the entity, then hope that the method is called at a later time

Are you sure? Effectively, you are introducing an ordering constraint on the domain model - none of these messages is permitted unless that one has been received first, which means you've got a race condition. See Udi Dahan's Race Conditions Don't Exist

More generally, an entity is valid or in valid based on whether or not it is going to be able to satisfy the post conditions of its methods, you can loosen the constraints during construction if the post conditions are broader.

Scott Wlaschin's Domain Modeling Made Functional describes this in detail; in summary, _periodDate might be an Or Type, and interacting with it has explicit choices: do this thing if valid, do this other thing if invalid.

The idea that constructing a ProductReturn requires a valid _periodDate isn't wrong; but there are tradeoffs to consider that will vary depending on the context you are operating in.

Finally, if any date is saved to the database that ISN’T the next sequential date, the calculation of subsequent returns will fail as we require a sequence to do the calculation properly.

If you have strong constraints between data stored here and data stored somewhere else, then this could indicate a modeling problem. Make sure you understand the implications of Set Validation before you get too deeply invested into a design.

VoiceOfUnreason
  • 52,766
  • 5
  • 49
  • 91
  • Thanks for this response, very detailed! The reason I’m wanting to do this in the aggregate instead of pass in, is because the logic of what the date should be (I feel) is business logic. The user can’t pass a date in, because at any given time, the date for a return can ONLY be the next date from the last return. I did notice the constructor anti pattern, and so I’m planning on creating a factory – Olorin Feb 02 '18 at 14:04
  • The other thing I wanted to avoid, was forcing the user to call multiple methods to set properties, when they can pass in the data that makes sense to construct the object, when they create it. In relation to the value computation, there are business rules about how it is calculated and when, which is why I felt using a service was more appropriate. I.e. sometimes the value will be hardcoded, other times it WILL need to be calculated based off other properties. Am I in a completely wrong mindset here? – Olorin Feb 02 '18 at 14:08
  • I should also mention - a return is invalid without a date, so I can’t construct the entity, then hope that the method is called at a later time – Olorin Feb 02 '18 at 14:11
  • (Sorry for the comment spam) but I’m also worried about the amount of times the return calculation service is required throughout setting methods in the aggregate. Each of these methods have an affect on how the return is calculated, and so it does require recalculation, but the way this is being handled just smells to me – Olorin Feb 02 '18 at 14:15
  • Finally, if any date is saved to the database that ISN’T the next sequential date, the calculation of subsequent returns will fail as we require a sequence to do the calculation properly. It’s a tricky one - the return doesn’t know how to get the next date, but it can’t be passed in directly as it may be incorrect. That’s why I opted for the domain service approach! – Olorin Feb 02 '18 at 14:21
  • _to pass in a service to compute a value, when you **could just compute the value and pass it in**._ Well, the bold part breaks encapsulation. Using double-dispatch pattern here (on a method, not constructor), ensures the encapsulation (injecting the service in the method, the method calls in and passes arguments it needs, then assigns the value). This makes it impossible to set any arbitrary value by calling `productReturn.SetReturnDate(calculatedDate)`. – Tseng Feb 02 '18 at 15:48
  • @VoiceOfUnreason Just to address your edited answer, the indication of a modelling problem is the part I’m worried about I think. It feels like the product returns should be in the product aggregate, but given the idea is this library will be used for a REST API, I can’t see how to get around the issue of having to load every single return when I create one (when I could get the information I need with simple queries). I definitely feel there’s a modelling issue, but I feel like there’s a trade off either way. – Olorin Feb 03 '18 at 00:19
  • @Tseng this was my thinking. I’m planning on creating a factory/factory method that will take those services in and initialise the date, as I feel that’s “more correct” – Olorin Feb 03 '18 at 00:20
  • @Olorin _but given the idea is this library will be used for a REST API,_ you must **IGNORE** any infrastructure concerns when designing your Domain and Ubiquitous language! Because if you start to consider the way its consumed (REST, desktop, Client-Server etc.), then you leak infrastructure concerns to your domain and you start designing your domain around infrastructure. In fact it should be the other way around. You create the domain first w/o any consideration about infrastructure, then you start doing the infrastructure and bend it the way to fit with the domain you designed earlier – Tseng Feb 03 '18 at 13:25
  • @Tseng I do agree with that, however, realistically I need to I’ve some thought to how I’m going to ensure this is all performing once it hits the REST API. I simply can’t be loading thousands of return records to create one, every time a new return is created. My current implementation was my way around this, but I agree it’s probably not the “correct” way of doing it – Olorin Feb 03 '18 at 21:44
0

Your problem is querying across multiple aggregates(product returns) for decision making(creating a new product return aggregate).

Decision making based on querying across aggregates using a repository will always be wrong; we will never be able to guarantee consistency, as the state read from the repository will always be a little old.(Aggregates are transaction boundaries. The state read from repository will only be correct at that very instant. In the next instant the aggregate's state may change.)

In your domain what I will do is create a ProductReturnManager AggregateRoot which manages returns for a particular product and a ProductReturn Aggregate which specify one specific return of the product. ProductReturnManager AggregateRoot manages the lifecycle of ProductReturnAggregate to ensure consistency.

The logic for assigning a next month sequential date to ProductReturn is in ProductReturnManager (basically ProductReturnManager act as a constructor). The behavior of product return will be in ProductReturnAggregate.

The ProductReturnManager can be modeled as a Saga, which is created on first CreateProductReturnCommand (for a productId), and same saga is loaded for further CreateProductReturn commands(correlated by productId). It handles ProductReturnCreatedEvent to update its state. Saga creation logic will be according to your business rules(Eg. saga creation is done on InvoiceRaisedForProduct event and handles CreateProductReturn commands.)

sample code:

ProductReturnManagerSagaState{

ProductId productId;
//can cache details about last product return  
ProductReturnDetails lastProductReturnDetails;

}

ProductReturnManagerSaga : Saga<ProductReturnManagerSagaState>,IAmStartedByMessages<CreateProductReturn>{

Handle(CreateProductReturn message){

//calculate next product return date
Date productReturnDate = getNextReturnDate(Data.lastProductReturnDetails.productReturnDate);

//create product return 
ProductReturnAggregateService.createProductReturn(Data.productId, productReturnDate);
}

Handle(ProductReturnCreatedEvent message){
//logic for updating last product return details in saga state

}

}

ProductReturnAggregate{

ProductId productId;
Date productReturnDate;
ProductPayment productPayment;
ProductReturnState productReturnState;

//commands for product return 
markProductReturnAsProcessing(); 
}

This is an excellent video by Udi Dahan on working across multiple aggregates.

  • Thanks for this answer. I think I had started to think down this track (that is abstracting the creation logic out of the aggregate), but didn’t have such a sophisticated solution. I am worried about having to mark the product return aggregate as “processing” though - doesn’t that imply the domain has some knowledge of how the persistence mechanism is working? That aside, I can see how this solution might work, but I was trying to avoid having to always load up at least one other return before creating another (and maintaining a list in memory), but maybe this is unavoidable – Olorin Feb 03 '18 at 00:25
  • @Olorin Do let us know the final approach taken by you to solve your problem. – Anisha Agrawal Feb 05 '18 at 11:01