Czy tutaj refactoryzacja ma sens?

0

Trochę się powtarzam niby, więc postanowiłem zrefactoryzować kod.

// PRZED: ------------------------------------------------------------

public class NipValidator implements ConstraintValidator<NipConstraint, String> {
 
    final int[] WAGES = {6, 5, 7, 2, 3, 4, 5, 6, 7};
    final int CHECK_SUM_MODULO = 11;
 
    @Override
    public boolean isValid(String value, ConstraintValidatorContext context) {
        return value.length() == 10 && value.matches("\\d{10}") && isValidCheckSum(value);
    }
 
    private boolean isValidCheckSum(String value) {
        String lastDigitStr = value.substring(value.length() - 1);
        return Integer.parseInt(lastDigitStr) == CheckSum.calculate(value, WAGES, CHECK_SUM_MODULO);
    }
}

.

public class PeselValidator implements ConstraintValidator<PeselConstraint, String> {
 
    final int[] WAGES = {9, 7, 3, 1, 9, 7, 3, 1, 9, 7};
    final int CHECK_SUM_MODULO = 10;
 
    @Override
    public boolean isValid(String value, ConstraintValidatorContext context) {
        return value.length() == 11 && value.matches("\\d{11}") && isValidCheckSum(value);
    }
 
    private boolean isValidCheckSum(String value) {
        String lastDigitStr = value.substring(value.length() - 1);
        return Integer.parseInt(lastDigitStr) == CheckSum.calculate(value, WAGES, CHECK_SUM_MODULO);
    }
}

// PO: ------------------------------------------------------------

public class Validator {

	private int[] wages;
	private int moduloCheckSum;
	private int length;
	private int idxCheckSumInValue;
	private String pattern;

	private Validator(Builder builder) {
		this.wages = builder.wages;
		this.moduloCheckSum = builder.moduloCheckSum;
		this.length = builder.length;
		this.idxCheckSumInValue = builder.idxCheckSumInValue;
		this.pattern = builder.pattern;
	}

	public boolean isValid(String value, ConstraintValidatorContext context) {
		return value.length() == length && value.matches(pattern) && isValidCheckSum(value);
	}

	private boolean isValidCheckSum(String value) {
		String lastDigitStr = value.substring(idxCheckSumInValue, idxCheckSumInValue+1);
		return Integer.parseInt(lastDigitStr) == CheckSum.calculate(value, wages, moduloCheckSum);
	}

	static class Builder {
		private int[] wages;
		private int moduloCheckSum;
		private int length;
		private int idxCheckSumInValue;
		private String pattern;

		Builder() {
			
		}
		
		Builder setWages(int[] wages) {
			this.wages = wages;
			return this;
		}
		Builder setModuloCheckSum(int moduloCheckSum) {
			this.moduloCheckSum = moduloCheckSum;
			return this;
		}
		Builder setLength(int length) {
			this.length = length;
			return this;
		}
		Builder setIdxCheckSumInValue(int idxCheckSumInValue) {
			this.idxCheckSumInValue = idxCheckSumInValue;
			return this;
		}
		Builder setPattern(String pattern) {
			this.pattern = pattern;
			return this;
		}

		Validator build() {
			return new Validator(this);
		}
	}
}

.

public class PeselValidator implements ConstraintValidator<PeselConstraint, String> {
	
	final Validator validator;
		
	public PeselValidator() {
		this.validator = new Validator.Builder()
				.setIdxCheckSumInValue(10)
				.setLength(11)
				.setModuloCheckSum(10)
				.setWages(new int[] {9, 7, 3, 1, 9, 7, 3, 1, 9, 7})
				.setPattern("\\d{11}")
				.build();
	}
	
	@Override
	public boolean isValid(String value, ConstraintValidatorContext context) {
		return validator.isValid(value, context);
	}
}

do tego druga taka sama klasa dla pesel jak dla nip.

w efekcie mam 2 razy więcej kodu niż miałem.

(wiem, że są gotowe constrainty do pesel i nip, ale chcę se swoje zrobić)

1
  1. Jakie są zalety wynikające z tej refaktoryzacji?

  2. Jakie są wady?

  3. weights, nie wages + checksum, a nie checkSum - nie bój się zerkać do słownika.

1

O, i to jest właśnie przykład refucktoringu.

1

Toż to jakaś masakra jest. Jak już chcesz tak kombinować to:

class LuhnValidator {
    private int[] weights;
    private int modulo;

    protected LuhnValidator(int[] weights, int modulo) {
        this.weights = weights;
        this.module = modulo;
    }

    public final boolean validate(final String value) {
        return value.length() == this.weights.length + 1 && isNumber(value) && isValidCheckSum(value, this.weights, this.modulo);
    }
 
    private static boolean isValidCheckSum(final String value, int[] weights, int modulo) {
        String lastDigitStr = value.substring(value.length() - 1);
        return Integer.parseInt(lastDigitStr) == CheckSum.calculate(value, weights, modulo);
    }

    private static boolean isNumber(final String string) {
        return string.chars().allMatch(c -> c >= '0' && c <= '9')
    }
}

Ale dla tak prostej rzeczy to IMHO nie ma sensu, bo to co było już jest dość czytelne.

1 użytkowników online, w tym zalogowanych: 0, gości: 1