1

Taking into consideration that I'm making an attempt at Domain-Driven Design and overall good separation of concerns when it comes to business logic, data access logic, etc...

Question: How do you validate a child object, when validation depends on one or more fields of a parent object? What about when the parent object's validation depends on the state of child objects?

Let's use a simple, contrived example: annual employee performance reviews.

John, a manager at his company, needs to complete a review for each of his subordinates. Collectively, all of the reviews John needs to perform this year are referred to as a Review Process. When John closes all of the reviews in his Review Process, the Review Process itself can be closed.

He's using my system to create, update, and close employee reviews. The system defines a ReviewProcess object, and it contains a List(Of Reviews) belonging to that Review Process, as follows:

Public Class ReviewProcess

    Public Property ReviewProcessID As Integer
    Public Property EmployeeID As Integer
    Public ReadOnly Property Reviews As List(Of Review)
    Public Property CloseDate As Date

    Public Sub Add(review As Review)

        _Reviews.Add(review)

    End Sub

End Class

Public Class Review

    Public Property ReviewID As Integer
    Public Property ReviewProcessID As Integer
    Public Property EmployeeID As Integer
    Public Property Rating As Integer
    Public Property Comments As String
    Public Property CloseDate As Date

End Class

John needs to add a new review to his Review Process. In the UI, he clicks Add, selects Steve's Employee ID, and clicks save:

Private Sub SaveClickEvent(sender As Object, e As EventArgs) Handles btnSave.Click

        ' _reviewProcess is a form-level variable, already instantiated when the form is opened.

        Dim newReview As New Review With {
            .ReviewProcessID = _reviewProcess.ReviewProcessID,
            .EmployeeID = DirectCast(cboEmployees.SelectedItem, Employee).EmployeeID
            }

        Dim service As New ReviewService
        Dim result As New ValidationResult

        result = service.Create(newReview)

        If result.IsValid Then

            _reviewProcess.Add(newReview)

        Else

            ' Show the validation errors to the user.

        End If

    End Sub

Inside the ReviewService.Create() function, the following code is found. A FluentValidation validator is instantiated to check the that the new Review is valid to save to the database:

Public Class ReviewService

    Public Function Create(item As Review) As ValidationResult

        Dim validator As New ReviewValidator
        Dim results As ValidationResult = validator.Validate(item, ruleSet:="OnSave")

        ' SE question: how/where do we validate that the parent ReviewProcess does not have a close date?

        If results.IsValid Then

            Dim repository As New ReviewDataAccess
            repository.Create(item)

        End If

        Return results

    End Function

End Class

For completeness, here is the ReviewValidator class:

Public Class ReviewValidator
    Inherits AbstractValidator(Of Review)

    Public Sub New()

        RuleSet("OnSave",
            Sub()
                RuleFor(Function(x) x.ReviewProcessID).NotEmpty
                RuleFor(Function(x) x.EmployeeID).NotEmpty
            End Sub
        )

        RuleSet("OnClose",
            Sub()
                RuleFor(Function(x) x.Rating).NotEmpty
                RuleFor(Function(x) x.Comments).NotEmpty
                RuleFor(Function(x) x.CloseDate).NotEmpty
            End Sub
        )

    End Sub

End Class

This is where I'm having a breakdown of DDD/OOP understanding. The architecture I have described above is fine, until I hit the business rule that "If a Review Process is closed, new Reviews cannot be added."

Repeating the question:

Question: How do you validate a child object, when validation depends on one or more fields of a parent object? What about when the parent object's validation depends on the state of child objects?

Sure, I can ensure that the UI doesn't enable the Add button when the _reviewProcess.CloseDate <> Nothing, but that's obviously not in line with separation of concerns. I want to ensure this business rule is in the right place. This is where my question lies. There is also a business rule that a Review Process cannot be closed unless all of the Reviews that it owns are also closed. How does this fit into my design, if at all? Maybe I have to rethink and redo the design (with your help)?

I understand that maybe my design is way off base. Feedback on that is welcome.

1

Well I don't know if this will suffice to be a satisfying answer to your question or not, if not I hope that it can still help in some way, shape or form!

Answer (sort of??)

So I am just going to come right out and espouse my personal opinion (being based off solid design principles) and say that I would STRONGLY recommend against the general design of a component, in which there is ever a child object which must have some direct knowledge of its own parent. I do say this knowing that there are a handful of very esoteric scenarios in which this sort of thing would be acceptable, but for the most part I think that this would be introducing a level of complexity and coupling which is antithesis to the whole idea of creating an inheritance structure to begin with.

SOLUTION

Now that being said, I would propose that in a scenario like you give as an example...

Let's use a simple, contrived example: Annual Employee Performance Reviews.

John, a manager at his company, needs to complete a review for each of his subordinates. >Collectively, all of the reviews John needs to perform this year are referred to as a >Review Process. When John closes all of the reviews in his Review Process, the Review >Process itself can be closed.

The proper solution, as far as your software design goes would be to employ the power of Events (assuming c# is used, although you could use some sort of callback for other languages without such first class features). This solution would look something like the following:

  1. When the ReviewProcess is created, it should register an event listener to the given event, something such as OnReviewClosed.
    • The ReviewProcess should also be tracking which Review object is raising the event (pass this information via event args). Just to ensure that you could rule out any edge cases where someone closes the same review several times and counts for all of them, etc.
  2. Add a handler to the Review class so that it knows what information should be included in the event, in other words, what information does the ReviewProcess need to be aware of on the event.
  3. Wire up the OnClick to trigger the Review.Complete() method which in turn would raise the event.

This sort of approach offers the following main benefits

  • There is no sort of coupling between the child and parents which is oriented in the reverse direction of the natural inheritance. This means in a more ideological sense, that the child object no longer has to concern itself with any business logic that should be SOLELY the concern of the parent. Since this is the whole reason to have the parent object in the first place and not just an array of the child objects. This will prevent many, MANY issues in the future when trying to expand on the code as a nice bonus !
  • Using the event method will also allow for you to VERY EASILY add additional business logic to the application which depends on the knowledge of when/where a Review object is marked as closed without having to modify any exiting code and run the risk of braking code elsewhere because of the weird backwards coupling.
  • Lastly, it will reduce the amount of "bloat logic" inside of the Review class which is not really the responsibility of the Review itself, but rather some tertiary service which it should belong to naturally!
| improve this answer | |
  • 2
    Events are ok if there are a dynamic amount of interested parties, or not all parties are known beforehand. It is not ok as a workaround for a design problem where the dependencies between objects are not clear. Using events in these cases actually obfuscates what depends on what. I believe your description is one of these cases. – Robert Bräutigam Nov 20 '19 at 17:45
  • @RobertBräutigam I don't know what you mean by "dependencies between objects are not clear", in this example case you have a ReviewProcess which has some ammount of child Review objects. You want to ensure that the ReviewProcess does not complete unless all of the child Review objects are marked as completed. So from a pure design standpoint, a Review would never need to know any information about the parent ReviewProcess, therefor having that relation would be meaningless. The only information required is by the ReviewProcess needing to know if the child Review are completed – Flipurbit Nov 20 '19 at 21:27
  • @Flipurbit "...a Review would never need to know any information about the parent ReviewProcess..." This isn't accurate. A Review needs to know if the parent Review Process is itself closed, because the child Review cannot be reopened for further editing if the parent Review Process is itself closed. This is the main point of my dilemma and my question. – HardCode Dec 5 '19 at 18:27
0

How do you validate a child object, when validation depends on one or more fields of a parent object?

You can't, not easily at least, as you've discovered, if you are assuming that OOP requires that object validation happens inside the internals object being validated, or against a ValidatorBase(Of T) where the single T is the main class being validated.

A Review object should be able to validate itself, and that can certainly happen inside some set of internal methods on the class. However, I think where you went wrong is the idea that ALL validation around a Review is handled in a ReviewValidator class that is constrained to accepting only a single Review as an input.

You'd said it yourself, in order to pass validation, a Review must self-validate, but it also needs to ensure that its Parent ReviewProcess is in a valid state as well. Therefor the simplest solution is that the ReviewValidator class must accept the parent ReviewProcess as another parameter before it can return its validation result.

Now, looking at your Validation code, it looks like you've boxed yourself into a corner where each business entity has a single Validator(of T). Just scratch that and accept that in the business rules in front of you, you will need a different kind of validator that accepts a Review and a ReviewProcess object, or perhaps fold them into a new Struct with one property for each and have a validator for that new struct.

| improve this answer | |
  • Does this represent the "aggregate root" term I've been seeing around? The ReviewProcess is the aggregate root, and just validate the aggregate root and make sure to expose the valid or invalid state of each child Review? – HardCode Jan 2 at 15:23
  • I'm not familiar with the term "aggregate root" other than having heard it in passing, but if the end result is that you call the validator function for the parent ReviewProcess then its fine I guess, but it makes me nervous, because each call to validate a Review (by calling the validator on the parent ReviewProcess) now validates all the sibling Review's of the target review (because the parent validator validates all the Review children). This might not be a problem now, but it smells like trouble down the road to me. – Graham Jan 2 at 21:12

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.