Design review

[Prasanna Raman]

I would like to get some reviews for the following code. The application generates payslips after computing tax based on tax slabs, given an annual income. I've posted one or two bits of this in a different post in this forum, but I would like to post the complete code for review here in one place.

package payslip;

import payslip.exception.InputFormatException;
import payslip.model.Pay;
import payslip.model.PayslipFrequency;
import payslip.model.PayslipGenerator;
import payslip.service.PayService;
import payslip.validators.InputValidator;

import java.util.Arrays;
import java.util.Scanner;

import static payslip.utils.InputSifterUtils.*;

class PaySlipGeneratorApplication {

    static void go() {
        generatePaySlip();
    }

    private static void generatePaySlip() {

         String input = "GenerateMonthlyPayslip \"Tom Harry\" 90000";

        if (new InputValidator().validate(input)) {
            new PayslipGenerator().generate(getName(input), getPay(input), getFrequency(input));
        } else {
            throw new InputFormatException("Please enter input in this format: \"GenerateMonthlyPayslip \\\"Tom Harry\\\" 60000\"");
        }
    }

    private static PayslipFrequency getFrequency(String input) {
        return Arrays.stream(PayslipFrequency.values())
                .filter(c -> c.getValue().equals(extractPayslipFrequencyFrom(input)))
                .findFirst().get();
    }

    private static Pay getPay(String input) {
        return PayService.calculatePay(extractSalaryFrom(input));
    }

    private static String getName(String input) {
        return extractNameFrom(input);
    }
}

package payslip.validators;

public interface Validator<T> {
    boolean validate(T name);
}

package payslip.validators;

public class NameValidator implements Validator<String> {
    /**
     * This method validates name to see if it matches the pattern
     * @param name
     * @return
     */
    public boolean validate(String name) {
        return name.matches("[a-zA-Z ]+");
    }
}

package payslip.validators;

import java.math.BigDecimal;

public class AnnualSalaryValidator implements Validator<BigDecimal> {

    /**
     * This method validates annual salary to see if it's a positive number
     * @param annualSalary
     * @return
     */
    public boolean validate(BigDecimal annualSalary) {
        return annualSalary.compareTo(BigDecimal.valueOf(0)) > 0;
    }
}

package payslip.validators;

import payslip.model.PayslipFrequency;

public class MonthlyPayslipFrequencyValidator implements Validator<String> {
    /**
     * This method validates payslip frequency to see if it's one of the allowed entries
     * @param payslipFrequency
     * @return
     */
    public boolean validate(String payslipFrequency) {
        return payslipFrequency.equalsIgnoreCase(PayslipFrequency.MONTHLY.getValue());
    }
}

package payslip.validators;

import payslip.utils.InputSifterUtils;

public class InputValidator implements Validator<String> {

    /**
     * This method validates input format
     * @param input
     * @return
     */
    public boolean validate(String input) {
        String regex = "([a-zA-Z]+) (\"[a-zA-Z ]+\") ([0-9]+)";
        return input.matches(regex) && new NameValidator().validate(InputSifterUtils.extractNameFrom(input))
                && new AnnualSalaryValidator().validate(InputSifterUtils.extractSalaryFrom(input))
                && new MonthlyPayslipFrequencyValidator().validate(InputSifterUtils.extractPayslipFrequencyFrom(input));
    }
}

package payslip.utils;

import java.math.BigDecimal;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class InputSifterUtils {

    private static Matcher getMatcher(String input) {
        Pattern pattern = Pattern.compile("([a-zA-Z]+) (\"[a-zA-Z ]+\") ([0-9]+)");
        return pattern.matcher(input);
    }

    /**
     * This method extracts salary from the input
     * @param input
     * @return
     */
    public static BigDecimal extractSalaryFrom(String input) {
        Matcher matcher = getMatcher(input);
        BigDecimal value = BigDecimal.valueOf(0);
        if (matcher.matches()) {
            value = new BigDecimal(matcher.group(3).trim());
        }
        return value;
    }

    /**
     * This method extracts name from the input
     * @param input
     * @return
     */
    public static String extractNameFrom(String input) {
        Matcher matcher = getMatcher(input);
        String value = "";
        if (matcher.matches()) {
            value = matcher.group(2).replaceAll("^\"|\"$", "");
        }
        return value;
    }

    /**
     * This method extracts payslip frequency from the input
     * @param input
     * @return
     */
    public static String extractPayslipFrequencyFrom(String input) {
        Matcher matcher = getMatcher(input);
        String value = "";
        if (matcher.matches()) {
            value = matcher.group(1).trim();
        }
        return value;
    }
}

package payslip.model;

import java.math.BigDecimal;

public class Pay {

    private BigDecimal monthlyIncomeTax;
    private BigDecimal grossMonthlyIncome;
    private BigDecimal netMonthlyIncome;

    public BigDecimal getGrossMonthlyIncome() {
        return grossMonthlyIncome;
    }

    public BigDecimal getNetMonthlyIncome() {
        return netMonthlyIncome;
    }

    public BigDecimal getMonthlyIncomeTax() {
        return monthlyIncomeTax;
    }

    public void setGrossMonthlyIncome(BigDecimal grossMonthlyIncome) {
        this.grossMonthlyIncome = grossMonthlyIncome;
    }

    public void setMonthlyIncomeTax(BigDecimal monthlyIncomeTax) {
        this.monthlyIncomeTax = monthlyIncomeTax;
    }

    public void setNetMonthlyIncome(BigDecimal netMonthlyIncome) {
        this.netMonthlyIncome = netMonthlyIncome;
    }
}

package payslip.model;

import java.math.RoundingMode;

public enum PayslipFrequency {

    MONTHLY("GenerateMonthlyPayslip") {
        @Override
        public void generate(String name, Pay pay) {
                System.out.println("Monthly Payslip for: " + "\"" + name + "\"");
                System.out.println("Gross Monthly Income: $" + pay.getGrossMonthlyIncome().setScale(0, RoundingMode.HALF_UP));
                System.out.println("Monthly Income Tax: $" + pay.getMonthlyIncomeTax().setScale(0, RoundingMode.HALF_UP));
                System.out.println("Net Monthly Income: $" + pay.getNetMonthlyIncome().setScale(0, RoundingMode.HALF_UP));
            }
    };

    private final String value;

    PayslipFrequency(String inputString) {
        this.value = inputString;
    }

    public String getValue() {
        return value;
    }

    public void generate(String name, Pay pay) {
    }
}

package payslip.model;

public class PayslipGenerator {

    public void generate(String name, Pay pay, PayslipFrequency payslipFrequency) {
       payslipFrequency.generate(name, pay);
    }
}

package payslip.service;

import payslip.model.Pay;

import java.math.BigDecimal;
import java.math.RoundingMode;

public class PayService {
    /**
     * This method calculates Pay based on salary
     * @param salary
     * @return
     */
    public static Pay calculatePay(BigDecimal salary) {
        BigDecimal monthsInAYear = BigDecimal.valueOf(12);
        Pay pay = new Pay();
        pay.setGrossMonthlyIncome(salary.divide(monthsInAYear, 2, RoundingMode.HALF_UP));
        pay.setMonthlyIncomeTax(TaxService.computeTax(salary));
        pay.setNetMonthlyIncome(pay.getGrossMonthlyIncome()
                .subtract(pay.getMonthlyIncomeTax()).setScale(2, RoundingMode.HALF_UP));
        return pay;
    }
}

package payslip.service;

import java.math.BigDecimal;
import java.math.RoundingMode;

class TaxService {

    // These hold the different tax rates
    private static final BigDecimal BASE_TAX_RATE = BigDecimal.valueOf(.1);
    private static final BigDecimal LOW_TAX_RATE = BigDecimal.valueOf(.2);
    private static final BigDecimal MEDIUM_TAX_RATE = BigDecimal.valueOf(.3);
    private static final BigDecimal HIGH_TAX_RATE = BigDecimal.valueOf(.4);

    // These hold the different threshold limits
    private static final BigDecimal BASE_THRESHOLD = BigDecimal.valueOf(20000);
    private static final BigDecimal LOW_THRESHOLD = BigDecimal.valueOf(40000);
    private static final BigDecimal MEDIUM_THRESHOLD = BigDecimal.valueOf(80000);
    private static final BigDecimal HIGH_THRESHOLD = BigDecimal.valueOf(180000);

    /**
     * This method computes the monthly income tax based on income
     *
     * @param income
     * @return
     */
    static BigDecimal computeTax(BigDecimal income) {

        BigDecimal numberOfMonths = BigDecimal.valueOf(12);

        if (incomeIsLessThanThreshold(income)) {
            return BigDecimal.valueOf(0);
        }

        BigDecimal lowRateTax = getTaxFor(income, BASE_THRESHOLD, BASE_TAX_RATE);
        BigDecimal mediumRateTax = getTaxFor(income, LOW_THRESHOLD, LOW_TAX_RATE.subtract(BASE_TAX_RATE));
        BigDecimal highRateTax = getTaxFor(income, MEDIUM_THRESHOLD, MEDIUM_TAX_RATE.subtract(LOW_TAX_RATE));
        BigDecimal topRateTax = getTaxFor(income, HIGH_THRESHOLD, HIGH_TAX_RATE.subtract(MEDIUM_TAX_RATE));

        return lowRateTax.add(mediumRateTax).add(highRateTax).add(topRateTax)
                .divide(numberOfMonths, 2, RoundingMode.HALF_UP);
    }

    private static BigDecimal getTaxFor(BigDecimal income, BigDecimal threshold, BigDecimal taxRate) {
        return BigDecimal.ZERO.max(income.subtract(threshold).multiply(taxRate));
    }

    private static boolean incomeIsLessThanThreshold(BigDecimal income) {
        return income.compareTo(BASE_THRESHOLD) < 0;
    }
}

[Prasanna Raman]

Here are some test classes:

package payslip.validators;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class NameValidatorTest {

    private NameValidator nameValidator = new NameValidator();

    @Test
    void shouldReturnTrueWhenNameIsValid() {
        assertTrue(nameValidator.validate("Mary Song"));
    }

    @Test
    void shouldReturnFalseWhenNameIsInvalid() {
        assertFalse(nameValidator.validate("Invalid na##me"));
    }
}

package payslip.validators;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class MonthlyPayslipFrequencyValidatorTest {

    private MonthlyPayslipFrequencyValidator monthlyPayslipFrequencyValidator = new MonthlyPayslipFrequencyValidator();

    @Test
    void shouldReturnTrueWhenFrequencyIsValid() {
        assertTrue(monthlyPayslipFrequencyValidator.validate("GenerateMonthlyPayslip"));
    }

    @Test
    void shouldReturnFalseWhenFrequencyIsInvalid() {
        assertFalse(monthlyPayslipFrequencyValidator.validate("GenerateWeeklyPayslip"));
    }
}

package payslip.validators;

import org.junit.jupiter.api.Test;

import java.math.BigDecimal;

import static org.junit.jupiter.api.Assertions.*;

class AnnualSalaryValidatorTest {

    private AnnualSalaryValidator annualSalaryValidator = new AnnualSalaryValidator();

    @Test
    void shouldReturnTrueWhenAnnualSalaryIsValid() {
        assertTrue(annualSalaryValidator.validate(BigDecimal.valueOf(50000)));
    }

    @Test
    void shouldReturnFalseWhenAnnualSalaryIsInvalid() {
        assertFalse(annualSalaryValidator.validate(BigDecimal.valueOf(-2000)));
    }
}

package payslip.validators;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class InputValidatorTest {

    private InputValidator inputValidator = new InputValidator();

    @Test
    void shouldReturnTrueWhenInputIsValid() {
        assertTrue(inputValidator.validate("GenerateMonthlyPayslip \"Mary Song\" 60000"));
    }

    @Test
    void shouldReturnFalseWhenInputIsInvalid() {
        assertFalse(inputValidator.validate("GenerateMonthlyPayslip \"Mary Song123\" 60000"));
    }
}

package payslip.utils;

import org.junit.jupiter.api.Test;

import java.math.BigDecimal;

import static org.junit.jupiter.api.Assertions.*;

class InputSifterUtilsTest {

    private String input = "GenerateMonthlyPayslip \"Mary Song\" 60000";

    @Test
    void shouldExtractNameFromInput() {
        String actual = InputSifterUtils.extractNameFrom(input);
        assertEquals("Mary Song", actual);
    }

    @Test
    void shouldExtractSalaryFromInput() {
        BigDecimal actual = InputSifterUtils.extractSalaryFrom(input);
        assertEquals(BigDecimal.valueOf(60000), actual);
    }

    @Test
    void shouldExtractPayslipFrequencyFromInput() {
        String actual = InputSifterUtils.extractPayslipFrequencyFrom(input);
        assertEquals("GenerateMonthlyPayslip", actual);
    }
}

package payslip.service;

import org.junit.jupiter.api.Test;
import payslip.model.Pay;

import java.math.BigDecimal;
import java.math.RoundingMode;

import static org.junit.jupiter.api.Assertions.*;

class PayServiceTest {

    @Test
    void shouldCalculatePay() {
        Pay actualPay = PayService.calculatePay(BigDecimal.valueOf(60000));
        assertEquals(BigDecimal.valueOf(5000).setScale(2, RoundingMode.HALF_UP), actualPay.getGrossMonthlyIncome());
        assertEquals(BigDecimal.valueOf(500).setScale(2, RoundingMode.HALF_UP), actualPay.getMonthlyIncomeTax());
        assertEquals(BigDecimal.valueOf(4500).setScale(2, RoundingMode.HALF_UP), actualPay.getNetMonthlyIncome());

    }

}

package payslip.service;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertEquals;

class TaxServiceTest {

    private TaxService subject = new TaxService();

    @ParameterizedTest
    @MethodSource("valueStream")
    void shouldComputeTax(BigDecimal input, BigDecimal expected) {
        BigDecimal actual = subject.computeTax(input);
        assertEquals(expected, actual);
    }

    static Stream<Arguments> valueStream() throws Throwable {
        return Stream.of(
                Arguments.of(BigDecimal.valueOf(170000), BigDecimal.valueOf(3083.33)),
                Arguments.of(BigDecimal.valueOf(80000), BigDecimal.valueOf(833.33)),
                Arguments.of(BigDecimal.valueOf(180000), BigDecimal.valueOf(3333.33)),
                Arguments.of(BigDecimal.valueOf(70000), BigDecimal.valueOf(666.67)),
                Arguments.of(BigDecimal.valueOf(200000), BigDecimal.valueOf(4000.00).setScale(2, RoundingMode.HALF_UP)),
                Arguments.of(BigDecimal.valueOf(60000), BigDecimal.valueOf(500.00).setScale(2, RoundingMode.HALF_UP))
        );
    }
}

[Campbell Ritchie]

Why isn't your validation method static?

[Prasanna Raman]

Campbell Ritchie wrote:Why isn't your validation method static?

Interface won't allow me to make it static.

[Campbell Ritchie]

Where are you using that interface? I can't see any Validator<XYZ> declarations anywhere. That casts doubt on the wisdom of having that interface at all.
Please check the regex in name validator carefully. I think it may not work. What happens if you feed, “  ” to it?

[Prasanna Raman]

Campbell Ritchie wrote:Where are you using that interface? I can't see any Validator<XYZ> declarations anywhere. That casts doubt on the wisdom of having that interface at all.
Please check the regex in name validator carefully. I think it may not work. What happens if you feed, “  ” to it?

All 3 validation classes implement that interface. Is that not a good thing to do?

Yes, the name validator will break in that case. I will correct that.

[Junilu Lacar]

First comment (haven't looked at anything else): the NameValidator tests are still lacking, in my opinion. They tell a very vague story. Basically, if it's valid, the validator returns true, false otherwise. Not very helpful. What makes an input valid or invalid? That should be very clear from reading the test but it's not. I see that "#" is apparently not allowed but is that all? Will it reject a name with any special character? Many people have hyphenated names so will it accept "Zeta-Jones," for example? What about ", Jr."? Again, the test just does not spell it out.

A better test name might be should_only_have_letters_and_space -- that tells the reader exactly what will be accepted and what will be rejected.

[Junilu Lacar]

What the name should_accept_only_letters_and_space also does is it makes you think of special cases that you might have missed. Would it be acceptable to have a name like "A B C D E F G"? That kind of string will be accepted by the validator.

[Knute Snortum]

One thing I noticed is that InputSifterUtils and InputValidator use the same String to compile a pattern.  I would rather put this near the top of InputSirterUtils:

public final static String patternString = "([a-zA-Z]+) (\"[a-zA-Z ]+\") ([0-9]+)"

and then use this in InputValidator instead of the local variable.

[Junilu Lacar]

PayslipFrequency is a strange-looking (smelly) enum. It only has one value and it's dependent on the Pay class.

[Prasanna Raman]

Junilu Lacar wrote:What the name should_accept_only_letters_and_space also does is it makes you think of special cases that you might have missed. Would it be acceptable to have a name like "A B C D E F G"? That kind of string will be accepted by the validator.

Should the name validate be changed to this? Or should I change the name of the variable to something like this?

I have the interface as validate, that's why I had to call this validate.

[Junilu Lacar]

I wasn't suggesting to change the method name from validate. Since that's an interface method, it should be generic enough to hide details specific to each implementation. It is the test method that doesn't give a specific enough story when it should.

It seems you didnt get what I meant in my first reply. It's not enough to say that validate returns true if valid and false if invalid. The test should make it clear exactly why you expect true or false to be the result. Reading the test code still makes me guess as to exactly why a name that happens to contain # would be rejected. Is it just # that gets rejected? What other characters would get rejected? Those questions should be answered by tests, not by having to go and read the production code.

[Prasanna Raman]

Thank you, Junilu. I've updated those. Is the name for the false case good enough? Or too wordy? What's the next thing I can look at? I intend to take this as an involved learning experience, so please point out whatever things you feel are not right. I will work on one thing at a time.

    @Test
    void should_accept_only_letters_and_space() {
        assertTrue(nameValidator.validate("Mary Song"));
    }

    @Test
    void should_not_accept_characters_other_than_letters_and_space() {
        assertFalse(nameValidator.validate("Invalid na##me"));
    }

Also, I can see that using underscores rather than camel case here has improved readability significantly. Is that OK to do for tests? I know Java follows camel case.

[Liutauras Vilda]

Prasanna Raman wrote:

public interface Validator<T> {
    boolean validate(T name);
}

Which implementation you had in mind when created this interface?

[Prasanna Raman]

Liutauras Vilda wrote:

Prasanna Raman wrote:

public interface Validator<T> {
    boolean validate(T name);
}

Which implementation you had in mind when created this interface?

Sorry, I don't understand what you mean by which implementation?

[Liutauras Vilda]

PaySlipGeneratorApplication

I don't like how the PaySlip is spelt. I'd prefer reading it PayslipGenerator, and I'd remove Application word from it. Whad kind of application that is if it contains only static methods inside?

As a result of static methods and no internal state of a class, you have such calls:

new PayslipGenerator().generate(getName(input), getPay(input), getFrequency(input));

As you see you pass the same variable 3 times: input, input, input to the same utility classe's methods.

[Liutauras Vilda]

Prasanna Raman wrote:

Liutauras Vilda wrote:

Prasanna Raman wrote:

public interface Validator<T> {
    boolean validate(T name);
}

Which implementation you had in mind when created this interface?

Sorry, I don't understand what you mean by which implementation?

Probably not everyone would find it important...

NameValidator you could relate well with interface's method parameter validate(T name), but for MonthlyPayslipFrequencyValidator when the input is really a frequency, argument "name" seems to be off. I'd have called it simply input in the interface's method's validate parameter.

[Prasanna Raman]

Liutauras Vilda wrote:

PaySlipGeneratorApplication

As you see you pass the same variable 3 times: input, input, input to the same utility classe's methods.

I've changed the spelling, but I don't know what to do about the class name, because there is another class named PayslipGenerator.

Also, how can I change it so that I don't have to pass the same input thrice?

[Prasanna Raman]

Liutauras Vilda wrote:

Prasanna Raman wrote:

Liutauras Vilda wrote:

Prasanna Raman wrote:

public interface Validator<T> {
    boolean validate(T name);
}

Which implementation you had in mind when created this interface?

Sorry, I don't understand what you mean by which implementation?

Probably not everyone would find it important...

NameValidator you could relate well with interface's method parameter validate(T name), but for MonthlyPayslipFrequencyValidator when the input is really a frequency, argument "name" seems to be off. I'd have called it simply input in the interface's method's validate parameter.

Yes, valid point. In fact, I did change it but I think I might have forgotten to save it. It didn't read natural for me either when I was looking at my classes so I remember changing it to input. I've changed it again now (and saved it too!)

[Tom Joe]

Unrelated, but do you use version control like Git ? Is this code from a book ?

[Prasanna Raman]

Tom Joe wrote:Unrelated, but do you use version control like Git ? Is this code from a book ?

No, it's not from a book. It's just my code on my computer. I am using IntelliJ which usually saves it but I may have undone it unintentionally while I was doing some refactoring.

[Liutauras Vilda]

Why PayslipFrequency enum has method generate()? That looks to me like incorrectly assigned responsibility.

[Prasanna Raman]

Liutauras Vilda wrote:Why PayslipFrequency enum has method generate()? That looks to me like incorrectly assigned responsibility.

Yes, I was debating this too but probably ended up making the wrong choice. I initially had it in the generate method itself of the PayslipGenerator class?

Should it be there? Also, how do I get rid of passing the same input multiple times?

[Junilu Lacar]

Prasanna Raman wrote:Is that OK to do for tests? I know Java follows camel case.

Yes, it's fine to do it for tests. If you're on a team that follows certain coding standards, be sure to discuss it with them though. This is a convention I first saw the author Neal Ford use and I liked it so much more than camel caps that I've used it ever since.

As I was saying before, the names are not only clear and exact about what the tests are trying to document, but they also help identify possible gaps in your tests. For example, does the validation accept all spaces? What about single words? And again, what about hyphenated names?

[Prasanna Raman]

Thank you Junilu. I'll keep this mind. Please comment on the next aspect you'd like me to look into.

[Prasanna Raman]

Campbell Ritchie wrote:Where are you using that interface? I can't see any Validator<XYZ> declarations anywhere. That casts doubt on the wisdom of having that interface at all.

Do you mean other than just implementing the interface? No, it's never used outside of that because I just directly used new AnnualSalary() etc.

[Campbell Ritchie]

Wouldn't it read something like this?Validator<BigDecimal> validator = new AnnualSalaryValidator<>();

[Prasanna Raman]

Campbell Ritchie wrote:Wouldn't it read something like this?Validator<BigDecimal> validator = new AnnualSalaryValidator<>();

Yes, but I've just used it like this:

return input.matches(regex) && new NameValidator().validate(InputSifterUtils.extractNameFrom(input))
               && new AnnualSalaryValidator().validate(InputSifterUtils.extractSalaryFrom(input))
               && new MonthlyPayslipFrequencyValidator().validate(InputSifterUtils.extractPayslipFrequencyFrom(input));
   }

[Junilu Lacar]

I would look over the rest of the tests and make them more specific. For example, the InputValidatorTest seems to overlap the NameValidator test in that it's detecting an invalid name. This in turn seems to overlap the InputSifterTest in that it seems the InputValidator needs to extract the Name in order to do its job. None of the tests are specific enough so you have to bounce from one class to another to fully understand what the test name seems to only hint at. Again, a generic name like "shouldReturnTrueWhenValid" is not useful and leaves too much to question.

[Junilu Lacar]

More specific feedback about the InputValidatorTest.

To reiterate, the main problem is lack of specificity. Basically the tests are saying "If the input is valid, return true, otherwise return false." Well, duh. But what makes the input invalid or valid? One thing you can only infer from looking at the tests is that "Mary Song123" looks like it's supposed to be invalid. Why isn't this in the NameValidatorTest then? The input looks like it's made up of three distinct values that need to be parsed out. Does the order matter? Does the spacing matter? Can there be a \n in between? Should the delimiter always be space? Should any item that contains space be delimited with double quote? I'd expect the tests to give examples that clearly spell out the answer to some of these questions.

[Prasanna Raman]

Thank you Junilu. Are these better?

NameValidatorTest:

    @Test
    void should_accept_only_letters_and_space() {
        assertTrue(nameValidator.validate("Mary Song"));
    }

    @Test
    void should_not_accept_characters_other_than_letters_and_space() {
        assertFalse(nameValidator.validate("Invalid na##me"));
    }

AnnualSalaryValidatorTest:

    @Test
    void should_accept_annual_salary_greater_than_zero() {
        assertTrue(annualSalaryValidator.validate(BigDecimal.valueOf(50000)));
    }

    @Test
    void should_not_accept_annual_salary_less_than_zero() {
        assertFalse(annualSalaryValidator.validate(BigDecimal.valueOf(-2000)));
    }

MonthlyPayslipFrequencyValidatorTest:

    @Test
    void should_accept_only_if_input_matches_exact_string() {
        assertTrue(monthlyPayslipFrequencyValidator.validate("GenerateMonthlyPayslip"));
    }

    @ParameterizedTest
    @MethodSource("invalidValueStream")
    void should_not_accept_if_input_does_not_match_exact_string(String invalidInput) {
        assertFalse(monthlyPayslipFrequencyValidator.validate(invalidInput));
    }

    static Stream<Arguments> invalidValueStream() throws Throwable {
        return Stream.of(
                Arguments.of("Generate Monthly Payslip"),
                Arguments.of("MonthlyPayslip"),
                Arguments.of("Monthly pay slip"),
                Arguments.of("Monthly-pay-slip")
        );
    }

InputValidatorTest:

    @ParameterizedTest
    @MethodSource("validValueStream")
    void should_accept_only_if_input_matches_format(String validInput) {
        assertTrue(inputValidator.validate(validInput));
    }

    @ParameterizedTest
    @MethodSource("invalidValueStream")
    void should_not_accept_if_input_does_not_match_format(String invalidInput) {
        assertFalse(inputValidator.validate(invalidInput));
    }

    static Stream<Arguments> validValueStream() throws Throwable {
        return Stream.of(
                Arguments.of("GenerateMonthlyPayslip \"Mary Song\" 60000"),
                Arguments.of("GenerateMonthlyPayslip \"Tim Song\" 120000"),
                Arguments.of("GenerateMonthlyPayslip \"Albert L\" 120000")
        );
    }

    static Stream<Arguments> invalidValueStream() throws Throwable {
        return Stream.of(
                Arguments.of("GenerateMonthlyPayslip \"Mary Song123\" 60000"),
                Arguments.of("GenerateMonthlyPayslip \"Mary-Song\" 60000"),
                Arguments.of("GenerateMonthlyPayslip \"Mary Song\" -1000"),
                Arguments.of("GeneratePayslip \"Mary Song\" 60000"),
                Arguments.of("Generate-Monthly-Payslip \"Mary Song\" 60000")
        );
    }

I don't like the fact that I am just repeating what it's in the individual tests in the InputValidatorTest, possible smell for bad design? But I am not sure how to solve that yet. I don't know if I should test just the format separately and then have the other validations separately or combine them like I've done in the InputValidator.

I'll wait for your inputs on these before I do any more refactoring.