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

Allow putting hints on empty required block #36

Closed
cies opened this issue Apr 14, 2022 · 5 comments · Fixed by #172
Closed

Allow putting hints on empty required block #36

cies opened this issue Apr 14, 2022 · 5 comments · Fixed by #172
Labels
enhancement New feature or request
Milestone

Comments

@cies
Copy link

cies commented Apr 14, 2022

From the README it is not clear how I put a hint on an empty required block.

    val validator = Validation<DealFormDto> {
      DealFormDto::closedBy required {}
    }

I get the "is required" default message when closedBy is not present. But how to set my own message?

This does not work:

    val validator = Validation<DealFormDto> {
      DealFormDto::closedBy required { hint "Closed by is missing" }
    }
@cies
Copy link
Author

cies commented Apr 19, 2022

We now use this custom validation this in order to specify the hint on a required property:

fun <T : Any?> ValidationBuilder<T>.required() = addConstraint("is required") {
  if (it == null) {
    false
  } else {
    when (it) {
      is String -> it.isNotBlank()
      is Collection<*> -> it.isNotEmpty()
      else -> true
    }
  }
}

Which we use like this:

      FormDto::estimatedAnnualSales { required() hint "Estimated sales is required" } // Note we control the error message
      FormDto::estimatedAnnualSales ifPresent { minimum(0) hint "Estimated sales cannot be negative" }

Where before we'd do something like this:

      FormDto::estimatedAnnualSales required { minimum(0) hint "Estimated sales cannot be negative" }
      // Note we have no control over the error given when `estimatedAnnualSales` is omitted, it will always be "is required"

@cies
Copy link
Author

cies commented Apr 19, 2022

And we now use this one as well:

/** Convenience method, same as `required()` but takes a name. */
fun <T : Any?> ValidationBuilder<T>.requiredAs(name: String) = addConstraint("{0} is required", name) {
  if (it == null) {
    false
  } else {
    when (it) {
      is String -> it.isNotBlank()
      is Collection<*> -> it.isNotEmpty()
      else -> true
    }
  }
}

@nlochschmidt
Copy link
Member

nlochschmidt commented May 3, 2022

Awesome that you found a solution that works using the existing extension points 👏

Which we use like this:

    FormDto::estimatedAnnualSales { required() hint "Estimated sales is required" } // Note we control the error message
    FormDto::estimatedAnnualSales ifPresent { minimum(0) hint "Estimated sales cannot be negative" }

I agree, that doesn't look very good to me

So additionally to the one design option you proposed there are at least two other design options that I think are interesting:

Option 1: Your proposed option

DealFormDto::closedBy required { hint "Closed by is missing" }

Option 2: Using optional hint parameter on required

DealFormDto::closedBy required(hint = "Closed by is missing") { }

Option 3: Using infix after the required block

DealFormDto::closedBy required { } hint "Closed by is missing"

I think Option 3 is most in line with the existing idioms and, might possibly be easier to implement as well.

@dhoepelman
Copy link
Collaborator

Option 3 makes sense. Currently many things return Unit, and they could be returning something along the lines of Constraint to which hints can be added. Alternatively, they return ValidationResultBuilder and there's a way to add hints to those. This might also be a way to solve #76

@dhoepelman dhoepelman added the enhancement New feature or request label May 10, 2024
@dhoepelman dhoepelman added this to the v0.9.0 milestone Nov 9, 2024
@dhoepelman dhoepelman changed the title How to put a hint on an empty required block? Allow putting hints on empty required block Nov 13, 2024
@dhoepelman dhoepelman modified the milestones: v0.9.0, v0.10.0 Nov 13, 2024
@dhoepelman
Copy link
Collaborator

dhoepelman commented Nov 16, 2024

API for this introduced in #172, option 1 turned out to be the only viable one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants