Czy tutaj refactoryzacja ma sens?

2018-10-27 11:19
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ć)

edytowany 5x, ostatnio: Julian_, 2018-10-27 11:29

Pozostało 580 znaków

2018-10-27 13:12
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.


edytowany 3x, ostatnio: Patryk27, 2018-10-27 13:13
zalety: nieco bardziej bezmyślnie można naklepać kolejne walidatory podobne do tych o peselu wady: 2 razy więcej kodu i trzeba skakać po klasach. - Julian_ 2018-10-27 21:48

Pozostało 580 znaków

2018-10-27 15:03
1

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


"HUMAN BEINGS MAKE LIFE SO INTERESTING. DO YOU KNOW, THAT IN A UNIVERSE SO FULL OF WONDERS, THEY HAVE MANAGED TO INVENT BOREDOM."

Pozostało 580 znaków

2018-10-27 15:37
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.

edytowany 1x, ostatnio: hauleth, 2018-10-27 15:37
a co jak dojdzie Ci do zwaldiowania VIN. Wtedy cyfra kontrolna nie jest ostatnia. - Julian_ 2018-10-27 18:16
@Julian_: VIN nie używa algorytmu Luhna, więc piszesz sobie osobny walidator do numeru VIN. - hauleth 2018-10-27 19:04
VIN ma bardzo podobną cyfrę kontrolną, też przemnażasz elementy numeru przez rozjne wartosci i sprawdzasz modulo. - Julian_ 2018-10-27 21:39
Ok, ale to nie jest algorytm Luhna tylko jakaś wariacja. A nie ma co komplikować przypadku ogólnego dla jednego przypadku szczególnego. - hauleth 2018-10-28 12:03
akurat tutaj będę miał też walidację VIN... - Julian_ 2018-10-28 13:18
Nie ma sensu robić uogólnień, jeśli nie da się ich zrobić. Kod z pierwszego posta jest prosty i czytelny, czego chcieć więcej? Co najwyżej może drażnić capslock w nazwach stałych. - somekind 2018-10-28 16:22

Pozostało 580 znaków

Liczba odpowiedzi na stronę

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