HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Simple Java password rule enforcing

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
enforcingsimplerulejavapassword

Problem


  • rule 1: length must be between 8 and 50 chars.



  • rule 2: at least 3 of the 4 following rules:



  • --------2a: at least one upper case



  • --------2b: at least one lower case



  • --------2c: at least one numeral



  • --------2d: at least one non-alphanumeric.



I have a function which will take in two Strings (the password and the confirmation) and then return a boolean depending on if it verified. It also sets a label on my JavaFX screen to the error or confirmation message. It works fine, but the code seems cumbersome. any thoughts?

public boolean validatePassword(String newPass, String confirmPass)
{
    if (newPass == null || 
        confirmPass == null || 
        newPass.length() == 0 || 
        confirmPass.length() == 0 )
    {
        responseLabel.setText("");
        return false;
    }

    boolean equalsRule = newPass.equals(confirmPass);
    boolean lengthRule = newPass.length() >= 8 && newPass.length() = 3 && equalsRule && lengthRule)
    {
        responseLabel.setText("Password set successfully.");
        responseLabel.setTextFill(Color.GREEN);
        return true;
    }
    else
    {
        if (!equalsRule)
        {
            responseLabel.setText("Passwords must match.");
        }
        else if (ruleCount < 3 || !lengthRule)
        {
            responseLabel.setText("Password does not follow instructions above.");
        }
        responseLabel.setTextFill(Color.RED);
        return false;
    }
}

Solution

I see at least one big problem in your method: there is no separation of concerns. Everything is mixed into this method: common rule for password double-checking, business rules for password validation and UI calls for user response.

Extracting the business logic

The business logic in your case if the password validation specific rules, as you named them rule1, rule2a, rule2b, rule2c and rule2d.
Everytime you will need a password validation, you will need these rules to validate a password, therefore I would extract these rules into a BO.

import java.util.function.Predicate;
import java.util.stream.Stream;

public final class Password {
    private final String value;
    private final Predicate rule;

    public Password(String value) {
        this.value = value;
        Predicate rule1 = s -> s.length() >= 8 && s.length()  rule2a = s -> !s.equals(s.toLowerCase());
        Predicate rule2b = s -> !s.equals(s.toUpperCase());
        Predicate rule2c = s -> s.codePoints().anyMatch(Character::isDigit);
        Predicate rule2d = s -> s.codePoints().anyMatch(i -> !Character.isAlphabetic(i));
        Predicate rule2 = s -> Stream.of(rule2a, rule2b, rule2c, rule2d)
                                             .filter(p -> p.test(s))
                                             .count() >= 3;
        this.rule = rule1.and(rule2);
    }

    public boolean isValid() {
        return rule.test(value);
    }
}


If the rules are likely to change, it would also be possible to refactor this code and take the "final predicate" as a constructor argument.

Extracting "standard" logic

In your case, it is only the rule that assert the password and the confirmation password are equals. This is not a business-specific rule but rather a commonly accepted rule everytime a user creates a password. Moreover it seems you have a "validation flow" defined (first check the equality between the two password, then check if the password in itself is valid) that can be extracted into a separate class

public final class PasswordInteractor {
    public ValidationResult validatePassword(String pwd, String confirmPwd) {
        if (!pwd.equals(confirmPwd)) {
            return new ValidationResult("Passwords must match.", false);
        }

        if (!new Password(pwd).isValid()) {
            return new ValidationResult("Password does not follow instructions above.", false);
        }

        return new ValidationResult("Password set successfully", true);
    }
}


I created a DTO ValidationResult to transmit to the upper layer a composite result since it can be more than true/false.

public final class ValidationResult {
    private final String message;
    private final boolean success;

    public ValidationResult(String message, boolean success) {
        this.message = message;
        this.success = success;
    }

    public String getMessage() {
        return message;
    }

    public boolean isSuccess() {
        return success;
    }
}


Extracting UI calls

Finally the UI calls must stay in the UI-part, namely into the JavaFX controller. Typically it could looks like this (simplified):

import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.paint.Color;

public final class PasswordController {
    @FXML
    private Label responseLabel;

    @FXML
    private TextField pwd;

    @FXML
    private TextField confirmPwd;

    public void validateForm() {
        // validate other parts of the form

        ValidationResult result = new PasswordInteractor().validatePassword(pwd.getText(), confirmPwd.getText());

        if (result.isSuccess()) {
            responseLabel.setTextFill(Color.GREEN);
        } else {
            responseLabel.setTextFill(Color.RED);
        }
        responseLabel.setText(result.getMessage());
    }
}

Code Snippets

import java.util.function.Predicate;
import java.util.stream.Stream;

public final class Password {
    private final String value;
    private final Predicate<String> rule;

    public Password(String value) {
        this.value = value;
        Predicate<String> rule1 = s -> s.length() >= 8 && s.length() <= 50;
        Predicate<String> rule2a = s -> !s.equals(s.toLowerCase());
        Predicate<String> rule2b = s -> !s.equals(s.toUpperCase());
        Predicate<String> rule2c = s -> s.codePoints().anyMatch(Character::isDigit);
        Predicate<String> rule2d = s -> s.codePoints().anyMatch(i -> !Character.isAlphabetic(i));
        Predicate<String> rule2 = s -> Stream.of(rule2a, rule2b, rule2c, rule2d)
                                             .filter(p -> p.test(s))
                                             .count() >= 3;
        this.rule = rule1.and(rule2);
    }

    public boolean isValid() {
        return rule.test(value);
    }
}
public final class PasswordInteractor {
    public ValidationResult validatePassword(String pwd, String confirmPwd) {
        if (!pwd.equals(confirmPwd)) {
            return new ValidationResult("Passwords must match.", false);
        }

        if (!new Password(pwd).isValid()) {
            return new ValidationResult("Password does not follow instructions above.", false);
        }

        return new ValidationResult("Password set successfully", true);
    }
}
public final class ValidationResult {
    private final String message;
    private final boolean success;

    public ValidationResult(String message, boolean success) {
        this.message = message;
        this.success = success;
    }

    public String getMessage() {
        return message;
    }

    public boolean isSuccess() {
        return success;
    }
}
import javafx.fxml.FXML;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.paint.Color;

public final class PasswordController {
    @FXML
    private Label responseLabel;

    @FXML
    private TextField pwd;

    @FXML
    private TextField confirmPwd;

    public void validateForm() {
        // validate other parts of the form

        ValidationResult result = new PasswordInteractor().validatePassword(pwd.getText(), confirmPwd.getText());

        if (result.isSuccess()) {
            responseLabel.setTextFill(Color.GREEN);
        } else {
            responseLabel.setTextFill(Color.RED);
        }
        responseLabel.setText(result.getMessage());
    }
}

Context

StackExchange Code Review Q#146721, answer score: 4

Revisions (0)

No revisions yet.