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

Add validation msg to Validate Func #1266

Merged
merged 3 commits into from Oct 11, 2022

Conversation

micheleissa
Copy link
Contributor

Closes #1198

  • Add a way to associate an exception message with Validate lambda.
  • Add Unit test to above functionality.

- Add a way to associate an exception message with Validate lambda.
- Add Unit test to above functionality.
@micheleissa micheleissa mentioned this pull request Mar 17, 2019
@JoshClose
Copy link
Owner

wondering if it is okay to use FluentAssertion lib

Does Assert.ThrowsException not work in this case?

I think instead of a string message, it should probably be a Func<string, string> so the user can use the field that failed validation in the message if they want. I was thinking if the whole row would be valuable, but that data should be passed along as part of the exception that is thrown.

@JoshClose
Copy link
Owner

Something like this:

public virtual new MemberMap<TClass, TMember> Validate(Func<string, bool> validateExpression, Func<string, string> validateMessageExpression)
{
	Data.ValidateExpression = (Expression<Func<string, bool>>)(field => validateExpression(field));
	Data.ValidateMessageExpression = (Expression<Func<string, string>>)(field => validateMessageExpression(field));

	return this;
}

@micheleissa
Copy link
Contributor Author

micheleissa commented Mar 20, 2019

Does Assert.ThroeException not work in this case?

Assert.ThrowException doesn't allow for thrown message inspection so it can't be asserted and that's why I used FluentAssertion I could do it with an ExtensionMethod but Ifind the lib is a more natural approach.
I don't see a use case for func<string,string>
The field is already a member of The exception itself and can be read/used. What would be a use case ? Can you give me an example?

@JoshClose
Copy link
Owner

I would think people would naturally want to put it in their message.

$"Validation failed: Field '{field}' cannot contain spaces."

@micheleissa
Copy link
Contributor Author

micheleissa commented Mar 20, 2019

I would think people would naturally want to put it in their message.

$"Validation failed: Field '{field}' cannot contain spaces."

I'll see what I can do, I'm trying now to figure out how to invoke that expression. But what would the expression achieve? like from your example the expression will execute and if evaluated not to true be passed along to the exception constructor like I did originally or something different?

- Instead of string as paramter pass Func<string,string>.
- Execute the Expression and use the result as exception message.
@micheleissa
Copy link
Contributor Author

micheleissa commented Mar 31, 2019

@JoshClose
I did the change. Could you take a look?
Thanks.

@micheleissa
Copy link
Contributor Author

bump?? @JoshClose

@cwadrupldijjit
Copy link

This could make life much easier for me. What's the status of this @micheleissa and @JoshClose?

@JoshClose
Copy link
Owner

It'll still be at least a few weeks. I'm very swamped with work.

@dylanbudgen
Copy link

Hi all, this feature would be super useful for me for too. Do you have any updates @JoshClose? I know a good software engineer is a busy software engineer.

@larkydoo
Copy link

I'm looking for a resolution to this as well. Any word?

@simplifirepyroneil
Copy link

As a work around, you can throw your own Exception type with its own message instead of returning false in your validation. That will get cauight and wrapped by CsvHelper in a CsvHelperException which will contain the context and the inner exception will be your exception, Catch the CsfHelperException and use the inner exception to get your message.

@JoshClose JoshClose merged commit 9a089f8 into JoshClose:master Oct 11, 2022
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

Successfully merging this pull request may close these issues.

Validate Message
6 participants