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

Generics for InRule #6

Merged
merged 4 commits into from
Jul 29, 2024
Merged

Conversation

timmattison
Copy link
Contributor

I just got bitten by the fact that this code doesn't work how most people would expect:

	optionsList := []string{"a", "b", "c"}

	// I've always felt that this was dangerous
	assert.Error(t, Validate("a", In(optionsList)))

I've made some changes that make this behavior stay the same (validating a list like this will return an error) but makes it more ergonomic to simply pass the list as a variadic like this:

	optionsList := []string{"a", "b", "c"}

	// I've always felt that this was dangerous
	assert.Error(t, Validate("a", In(optionsList)))

	// This is better, no need to convert to interface
	assert.NoError(t, Validate("a", In(optionsList...)))

Without this I had been using a helper function like this:

func validationIn[T any](input []T) validation.InRule {
	interfaceValues := make([]interface{}, len(input))

	for i, v := range input {
		interfaceValues[i] = v
	}

	return validation.In(interfaceValues...)
}

But considering everyone would need to add this function if they want to validate against a list I think it would be nice to have it incorporated into the library.

Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great idea! Thanks. I appreciate this is from a while back, sorry! I'll see if I can get this merged ASAP.

validation_test.go Outdated Show resolved Hide resolved
@samlown
Copy link
Contributor

samlown commented Jul 29, 2024

Very nice. I had an error in some of my code where I was using the validation.In() method without any parameters, but the fix of course was to switch that to a validation.Empty.

Thanks!

@samlown samlown merged commit a36a506 into invopop:main Jul 29, 2024
2 checks passed
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.

2 participants