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

Introduce ValidationState #438

Closed
hohwille opened this issue Jul 2, 2024 · 1 comment · Fixed by #569
Closed

Introduce ValidationState #438

hohwille opened this issue Jul 2, 2024 · 1 comment · Fixed by #569
Assignees
Labels
enhancement New feature or request

Comments

@hohwille
Copy link
Member

hohwille commented Jul 2, 2024

Currently our validation is not implemented well:

public boolean validate() {
// avoid validation exception if not a candidate to be run.
for (Property<?> property : this.propertiesList) {
if (property.isRequired() && (property.getValue() == null)) {
return false;
}
}
for (Property<?> property : this.propertiesList) {
if (!property.validate()) {
return false;
}
}
return true;
}

public boolean validate() {
if (this.required && (getValue() == null)) {
return false;
}
if (this.validator != null) {
for (V value : this.value) {
this.validator.accept(value);
}
}
return true;
}

So we have validation result as boolean and also the concept to throw an exception if the validation fails in a properties validator.
Both approaches are not well designed.

Instead we should create an interface ValidationResult

public interface ValidationResult {
  boolean isValid();

  String getErrorMessage();
}

Then we can create an implementation ValidationState:

public class ValidationState implements ValidationResult {
  private final StringBuilder errorMessage;
  
  public boolean isValid() {
    return (this.errorMessage == null);
  }
  
  public String getErrorMessage() {
    if (this.errorMessage == null) {
      return null;
    }
    return this.errorMessage.toString();
  }

  public void addErrorMessage(String error) {
    if (this.errorMessage == null) {
      this.errorMessage = new StringBuilder(error.length());
    } else {
      this.errorMessage.append('\n');
    }
    this.errorMessage.append(error);
  }
}

This is just a design suggestion as starting point. We can also have a immutable implementation(s) of ValidationResult and have a ValidationResult toResult() method in ValidationState.

The final idea is that internal validation methods will take a ValidationState object as argument. This also applies for the validators of the Property classes. Here we need to change from Consumer<V> to a custom interface:

@FunctionalInterface
public interface PropertyValidator<V> {
  void validate(V value, ValidationState validationState);
}

The main API should take no argument and return ValidationResult.

An alternative approach is to skip the ValidationState and always return ValidationState and then add some aggregation logic that can compose multiple ValidationState objects into one. IMHO the latter approach is slightly more complex.

@hohwille hohwille added the enhancement New feature or request label Jul 2, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in IDEasy board Jul 2, 2024
@hohwille hohwille moved this from 🆕 New to 📋 Backlog in IDEasy board Jul 9, 2024
@hohwille hohwille moved this from 📋 Backlog to 🆕 New in IDEasy board Aug 13, 2024
@slskiba slskiba self-assigned this Aug 26, 2024
@slskiba slskiba moved this from 🆕 New to 🏗 In progress in IDEasy board Aug 26, 2024
hohwille pushed a commit that referenced this issue Sep 9, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in IDEasy board Sep 9, 2024
@hohwille hohwille added this to the release:2024.09.002 milestone Sep 9, 2024
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
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants