Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bridge Pattern for handling ToString formatting and Validations #19

Open
no1melman opened this issue Aug 9, 2021 · 4 comments
Open

Comments

@no1melman
Copy link

I'm super interest in this project, and there is some great stuff here - the simplicity is amazing.

Two things that jump out at me. First is the Validation flow. Validation is obviously super important to ValueObjects to ensure correctness, however the examples show throwing exceptions, which makes sense when provided void Validate() as signature.

Has anyone looked at possible providing an interface to allow a different approach to handling validation errors - or even better an approach using OneOf?

Second thing that stands out is the ToString, I love that it can be overrided to allow you to better display your data, I was wondering if there was an overload available using the IFormatter to allow reusability and culture specific formatting.

Interestingly these two could follow the same the strand, using the bridge pattern in order to surface more interopability.

I do guess however, that making the validation change, it would be harder to stop people using an invalid ValueObject because now it won't throw if something is wrong.

Finally are there any extension libraries to help users serialise these valueobjects to JSON, by overriding the jsonserialiser?

If there are some reasoning behind choosing the above approach, it may be worth adding them to the Readme - which I'm happy to do.

@SteveDunn
Copy link

SteveDunn commented Sep 5, 2021

I started something similar to this library a few years back and recently put it on GitHub and NuGet.
It started out life with a few key differences, namely:

  • validation was compulsory - a delegate had to be provided in the constructor of each value object for validation
  • the validation delegate returned a string saying what was wrong with the instance (or empty if it was OK) - the base class then threw an exception (to stop people accidentally forgetting to throw)
  • there was no way to create a default instance (thereby allowing a potentially invalid value creeping in)
  • it was serialisable via Newtonsoft.Json
  • it just represented a single underlying type (int, decimal, string etc.)

I introduced this to where I'm working and the feedback was not to use delegates in the constructor, instead provide a Validate method. The down-side to this is that (without some slow reflection), it's impossible to stop people using default constructors (new Age(); instead of Age.From(42)). So that's the version I have in GitHub, which is now very similar to this. It also persists via System.Text.Json too.

I've been searching for a way to make this bullet-proof so that it's impossible to create a valueobject bypassing validation, but I'm coming to the conclusion that it's not possible.
I'm also coming to the conclusion that codegen is probably the best way to fulfill these requirements.

@no1melman
Copy link
Author

I'm also coming to the conclusion that codegen is probably the best way to fulfill these requirements.

Yes I have been thinking about that as well...

validation was compulsory - a delegate had to be provided in the constructor of each value object for validation

I actually really like this idea... any more reasons it was rejected?

@mcintyre321
Copy link
Owner

mcintyre321 commented Sep 6, 2021 via email

@SteveDunn
Copy link

I'm also coming to the conclusion that codegen is probably the best way to fulfill these requirements.

Yes I have been thinking about that as well...

validation was compulsory - a delegate had to be provided in the constructor of each value object for validation

I actually really like this idea... any more reasons it was rejected?

It was rejected because of the extra heap allocation. This is partly understandable: if you previously had a struct and now it's a class, that's one heap allocation, and if you then have a delegate passed to it, that's another, and if they're on particularly hot paths, then it may cause issues. My only answer to this was that Value Objects on their own aren't much use and that they're normally contained in a long-lived type, e.g. Price would be part of Product; Age would be part of Person etc., and those long-lived object would likely be on the heap anyway, so the original struct would also likely be on the heap.

In my (as yet imaginary) generated code version, it'll allow users to use a struct or class (but not record)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants