michaellperry / Assisticant

MIT License
36 stars 19 forks source link

beginnings of fluent arcs out & back from ValidationRules class #23

Closed floyd-may closed 7 years ago

floyd-may commented 7 years ago

Hopefully this is a bit of a start in the right direction.

floyd-may commented 7 years ago

This should be a reasonable solution for the discussion in #22.

michaellperry commented 7 years ago

This syntax requires that I re-express a property in order to supply multiple rules. For example, see the branch pr23-reexpress. (Sorry, I don't know how to comment on a PR with another change.)

I'd like to use the language semantics to represent the hierarchy, and not rely upon the user to indent expressions that are syntactically on the same level.

floyd-may commented 7 years ago

This syntax requires that I re-express a property in order to supply multiple rules.

Let me chew on that for a bit. I've got a couple ideas but it might require decoupling the ValidationRules class from the fluent stuff. Are you opposed to having a .Build() at the end of the chain?

michaellperry commented 7 years ago

Not opposed at all.

floyd-may commented 7 years ago

I'd like to use the language semantics to represent the hierarchy, and not rely upon the user to indent expressions that are syntactically on the same level.

Understood. Indentation isn't a "must" but it can be helpful. I'm thinking along the same lines as what Fluent Migrator does. When I write Fluent Migrator code, I indent the subsequent fluent calls corresponding to each column because I hate long lines.

floyd-may commented 7 years ago

Okay, @michaellperry, how does this version grab you? You can chain on as many different validation rules as you like, each with their own message, to a single property.

michaellperry commented 7 years ago

Very nice. Your use of inheritance with PropertyValidationContext lets the graph return home at any time, yet remain type safe while configuring a property.

Let me build the NuGet package and kick the tires a bit. If it all looks good in practice, there are a couple of features I'd like to add:

I think I can get both of these with the same syntax and some clever use of your PropertyRuleset collections.

michaellperry commented 7 years ago

Noticed a couple of things:

I think the first is easily fixable. The second one might be something we just have to live with. I'll try a few things.

michaellperry commented 7 years ago

I opened a PR on your PR with what I found. Replacing the derived classes with extension methods gives us both benefits. I was frankly surprised that it worked!

Now it's just a simple matter of making the .WithMessage optional and providing a default message.

floyd-may commented 7 years ago

I opened a PR on your PR

LOL that's awesome! image