konform-kt / konform

Portable validations for Kotlin
https://www.konform.io
MIT License
680 stars 41 forks source link

Improve `minimum`, `maximum`… number constraints? #180

Closed pmeinhardt closed 1 day ago

pmeinhardt commented 4 days ago

Heya. Thanks for this neat library. 💚

I discovered that the current number constraints all convert to Double. This might be problematic for Number types such as BigInteger or BigDecimal.

An option might be to implement these constraints on Comparable<T>. 💭 This would mean, that these constraints will work for more than just Number types.

On the other hand, it might mean that the constraints will no longer work across different Number types. E.g., you might no longer be able to constrain an Int field with a Float, but maybe that's a good thing? 🤔

Happy to hear your thoughts. 🙂

Kind regards, Paul

dhoepelman commented 4 days ago

Good point about the toDouble, that's not corect for Long everywhere, or BigInteger and BigDecimal on the JVM.

From a technical perspective Comparable sounds very appealing, but I don't like the re-using the number constraints, because it would lead to this:

Validation<String> {
  minimum("abc")
}

On option is adding atLeast, greaterThan, lessThan, and atMost, which looks better for non-numbers:

Validation<String> {
  atLeast("abc")
}

not sure what's best here, at it would introduce a second name for the same thing.

you might no longer be able to constrain an Int field with a Float, but maybe that's a good thing?

While a small breaking change, I think it's a good thing. Generally you want to compare 2 of the same type. The exception here is Double and Long with Int argument, which is how I expect most of them are written now. This can be solved by providing overloads with Int param so as to not break code like this:

Validation<Double> {
  minimum(10)
}
pmeinhardt commented 1 day ago

Thank you @dhoepelman! 💚