Introduced TryValidate<T>() extension method

[klym1]

...for HttpRequest which accompanies Validate<T>(), but is more flexible, because it allows to explicitly handle the case when the model's validator is missing.

[jchannon]

Thanks!

I’m not against this but it has made me think. why would you want a Boolean when you already have it in the Validate method via the returned object IsValid property?

I wonder if Carter should throw if no validator is found instead?

On Sat, 28 Dec 2019 at 16:56, Mykola Klymyuk notifications@github.com wrote:

...for HttpRequest which accompanies Validate(), but is more flexible, because it allows to explicitly handle the case when the model's validator is missing.

You can view, comment on, or merge this pull request online at:

https://github.com/CarterCommunity/Carter/pull/230 Commit Summary

• Introduced TryValidate() extension method for HttpRequest which accompanies Validate(), but is more flexible, because it allows to explicitly handle the case when model validator is missing.

File Changes

• M src/Carter/ModelBinding/ValidationExtensions.cs https://github.com/CarterCommunity/Carter/pull/230/files#diff-0 (23)

Patch Links:

• https://github.com/CarterCommunity/Carter/pull/230.patch
• https://github.com/CarterCommunity/Carter/pull/230.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/230?email_source=notifications&email_token=AAAZVJTTS7TM3N6UCG26BZLQ26AKDA5CNFSM4KAPMCH2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IDBG3DQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJR6ZW2Z5PKGOAIID3TQ26AKDANCNFSM4KAPMCHQ .

[klym1]

why would you want a Boolean when you already have it in the Validate method via the returned object IsValid property

The answer is - separation of concerns, aka SRP. There are actually 2 quite distinct cases - when a validator exists for a model and when it does not. In the first case, we can tell what the validation result is because we can take the validator and check the model against it. In the second case, we can't get a validation result at all (there's no validator to check against). Therefore it would be wrong to claim IsValid=false because there's no result.

That's why these 2 paths must be handled differently.

I wonder if Carter should throw if no validator is found instead?

Back in Nancy times - a model with a missing validator would be counted as a valid model, so I think better to keep such behavior in Carter. Also, with Try semantics we can leave that decision up to end-users.

For example, users might choose not to throw an exception, but rather return an error (which is preferable in most cases):

//if no validator exists - skip the section, count a model as valid
if (req.TryValidate(model, out var validationResult) && !validationResult.IsValid)
{
    // validator exists, the model is not valid, return the error
}

[jchannon]

At the moment Carter doesn’t return a valid result on no validator found. Can’t remember how Nancy did it. Would need to look.

Think Carter should throw actually and we can have the TryValidate that returns false.

Would need to alter and add some tests for this PR

On Sat, 28 Dec 2019 at 18:20, Mykola Klymyuk notifications@github.com wrote:

why would you want a Boolean when you already have it in the Validate method via the returned object IsValid property

The answer is - separation of concerns, aka SRP. There are actually 2 quite distinct cases - when a validator exists for a model and when it does not. In the first case, we can tell what the validation result is because we can take the validator and check the model against it. In the second case, we can't get a validation result at all (there's no validator to check against). Therefore it would be wrong to claim IsValid=false because there's no result.

That's why these 2 paths must be handled differently.

I wonder if Carter should throw if no validator is found instead?

Back in Nancy times - a model with a missing validator would be counted as a valid model, so I think better to keep such behavior in Carter. Also, with Try semantics we can leave that decision up to end-users.

For example, users might choose not to throw an exception, but rather return an error (which is preferable in most cases):

//if no validator exists - skip the section, count a model as validif (req.TryValidate(model, out var validationResult) && !validationResult.IsValid) { // validator exists, the model is not valid, return the error }

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/230?email_source=notifications&email_token=AAAZVJQSBV72YJ2NUR77HMTQ26KGLA5CNFSM4KAPMCH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYPPIA#issuecomment-569440160, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJT3L4USUZ7FSYXXPVDQ26KGLANCNFSM4KAPMCHQ .

[klym1]

At the moment Carter doesn’t return a valid result on no validator found

And that's a bad approach. Let me explain why.

Consider two scenarios: 1) typical scenario: Nancy project migrating to Carter. It uses Fluent validator, but not all the models have validators (that works perfectly with Nancy). In order to make the project running with Carter developers would have either to create at least AbstractValidator<> stubs for all the models without validators or write own code which would pass the validation when there's no validator.

2) A new project built on top of Carter. It's been under active development and then developers decide to use FluentValidator. But they're facing the same dilemma: write validators for all the models at once or write their own code that would wrap the validation logic.

[jchannon]

But why call Validate when you have no validator to validate the model

On Sat, 28 Dec 2019 at 20:37, Mykola Klymyuk notifications@github.com wrote:

At the moment Carter doesn’t return a valid result on no validator found And that's a bad approach. Let me explain why.

Consider two scenarios:

1.

typical scenario: Nancy project migrating to Carter. It uses Fluent validator, but not all the models have validators (that works perfectly with Nancy). In order to make the project running with Carter developers would have either to create at least AbstractValidator<> stubs for all the models without validators or write own code which would pass the validation when there's no validator. 2.

A new project built on top of Carter. It's been under active development and then developers decide to use FluentValidator. But they're facing the same dilemma: write validators for all the models at once or write their own code that would wrap the validation logic.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/pull/230?email_source=notifications&email_token=AAAZVJRNJLKCWPRFQ4VXISTQ262JXA5CNFSM4KAPMCH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYRXIY#issuecomment-569449379, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJW7RBE35WK4V2FGYULQ262JXANCNFSM4KAPMCHQ .

[klym1]

Validating a model is a very boilerplate-ish operation, i.e. it requires a very similar code to be written in every handler, so very often it is moved to the infrastructure level, where a framework would call it for every handler. And I'm not sure if it's possible to find out whether there's a validator for a model by using e.g. FluentValidator capabilities (even if it is, I'd still prefer to have some consistent mechanism for that (like TryValidate<>()), that would work consistently across other validation libs).

[jchannon]

Thanks!

[jchannon]

Just thought this through some more and if you don't want validate to return a failure don't call it. If you have copy/pasted code that has BindAndValidate don't call it, just call Bind

[jchannon]

Have reverted the code.