Como é uma classe teórica (sem um requisito real), vou tomar como premissa que a responsabilidade dela é representar um CPF (e não somente validar ou formatar, pois aí algumas coisas poderiam ser mudadas, conforme discutido mais abaixo). Ou seja, eu passo uma String
contendo um possível número de CPF e ela só cria a instância se esse número for válido (sendo que "válido" considera apenas se os dígitos verificadores estão corretos, caso existam - se não existir, a classe calcula automaticamente).
Se a ideia é somente essa, não precisa de regex, pois ela só está servindo para verificar se a String
tem uma determinada quantidade de dígitos. Mas você pode fazer essa verificação durante o cálculo dos dígitos verificadores, economizando assim um loop desnecessário (a regex terá que varrer toda a String
para verificar que ela só tem dígitos). Na verdade dá para economizar 2 loops (o da regex e um dos que calculam os dígitos verificadores), pois ambos os dígitos verificadores podem ser calculados no mesmo loop. Uma alternativa para o construtor e os métodos auxiliares usados por ele seria:
public Cpf(String cpf) {
if (cpf == null) {
throw new IllegalArgumentException("CPF não pode ser nulo.");
} else if (cpf.length() != NRO_DIGITOS_COM_DV && cpf.length() != NRO_DIGITOS_SEM_DV) {
throw new IllegalArgumentException("Número de CPF deve ter " + NRO_DIGITOS_SEM_DV + " ou " + NRO_DIGITOS_COM_DV + " dígitos");
}
this.cpf = verificaDigitos(cpf);
}
private static String verificaDigitos(String cpf) { // este método só é chamado depois que eu já sei que o tamanho da string é 9 ou 11
int total1 = 0, total2 = 0;
int multiplicador = 10;
for (int i = 0; i < NRO_DIGITOS_SEM_DV; i++) { // percorre somente os 9 primeiros dígitos
int c = cpf.charAt(i) - 48;
if (c < 0 || c > 9) { // não é dígito de 0 a 9, inválido
throw new IllegalArgumentException("CPF deve ter somente dígitos: caractere inválido na posição " + i);
}
total1 += multiplicador * c;
total2 += (multiplicador + 1) * c;
multiplicador--;
}
int dv1 = dv(total1, 1, cpf);
int dv2 = dv(total2 + dv1 * 2, 2, cpf);
if (cpf.length() == NRO_DIGITOS_SEM_DV) { // tem 9 dígitos, concatenar os dígitos verificadores
return cpf + dv1 + dv2;
}
return cpf; // se chegou aqui é porque tem 11 dígitos e os dígitos verificadores são válidos, então retorna sem modificações
}
// recebe o total, o número do dígito verificador (i pode ser 1 ou 2) e a string do CPF
// se o tamanho da string é 11, verifica se o dígito verificador calculado é o mesmo que está na string
private static int dv(int total, int i, String cpf) {
int resto = total % 11;
int dv = resto < 2 ? 0 : 11 - resto;
if (cpf.length() == NRO_DIGITOS_COM_DV && dv != cpf.charAt(NRO_DIGITOS_COM_DV - (3 - i)) - 48)
throw new IllegalArgumentException((i == 1 ? "Primeiro" : "Segundo") + " dígito verificador não bate");
return dv;
}
No método verificaDigitos
eu faço apenas um loop pelos primeiros 9 dígitos, calculando as somatórias.
Um detalhe/curiosidade é que dentro do loop eu também poderia fazer assim:
if (i != 0) {
total2 += (multiplicador + 1) * c;
}
Pode parecer estranho pular a multiplicação do primeiro dígito por 11 em total2
, mas no final vamos calcular o resto da divisão por 11 e portanto o primeiro termo é redundante: se eu tiver um número n
e fizer n % 11
, o resultado será o mesmo que (n + (11 * x)) % 11
(para qualquer x
inteiro). Somar qualquer múltiplo de 11 não mudará o resto da divisão por 11, então somar "11 vezes o primeiro dígito" em total2
não faz diferença (se bem que nesse caso, talvez esse if
não passe de micro-otimização).
Depois de terminado o loop, eu vejo se o primeiro dígito verificador é igual ao do CPF informado, mas só se o tamanho da String
for 11 (se for 9, não tem o que verificar). Se for válido, eu prossigo para o segundo dígito. Repare que no if
do método dv
eu nem preciso verificar se o décimo e décimo primeiro caracteres são dígitos de 0 a 9, pois se não forem, o valor com certeza não baterá com os dígitos verificadores.
Por fim, eu verifico se devo concatenar os dígitos no final da String
(o que só deve ser feito se o tamanho dela for 9).
Com isso, o método que cria um CPF aleatório pode simplesmente trabalhar com strings de 9 dígitos (já que os dígitos verificadores serão calculados e concatenados dentro do próprio construtor):
public static Cpf criarAleatorio() {
String noveDigitosAleatorios = String.valueOf(ThreadLocalRandom.current().nextInt(100_000_000, 999_999_999 + 1));
return new Cpf(noveDigitosAleatorios);
}
Como nesse caso específico eu já sei que a String
terá apenas dígitos, talvez valesse a pena refatorar a classe para separar a validação do cálculo dos dígitos verificadores, pois neste caso não preciso verificar que todos os caracteres são dígitos - fica como "exercício para o leitor" :-).
Quanto à adoção de uma classe para isso, depende do objetivo. Você quer que a classe represente um CPF, ou só quer um validador/calculador de dígitos verificadores? Se fosse o segundo caso, faria mais sentido uma classe utilitária com métodos públicos e estáticos, que retornam apenas true
/false
(CPF válido ou inválido), ou se quiser ser mais específico, retornam códigos de erro que indiquem qual foi o problema (não tem dígitos, quantidade errada, dígitos verificadores não batem, etc). Outra alternativa seria encapsular esse código de erro em uma classe ou enum
específico (CpfValidationResult
?) caso valha a pena criar tal abstração (como é uma classe teórica sem requisito real, fica difícil opinar sobre o que é mais adequado).
Porém, se a ideia é que a classe represente um CPF, então eu acho que tudo bem lançar a exceção caso o CPF seja inválido (já que não parece fazer sentido criar uma instância se o número não é válido, pois aí ela deixaria de representar um CPF de fato - mas novamente, sem requisitos reais, não dá pra saber, vai que tem um requisito que permite ter "CPF's" com números inválidos, sei lá). Mas se fosse um validador, eu usaria uma das opções do parágrafo anterior, em vez de lançar uma exceção.
Por fim, o código nem ficou tão mais curto que o original, mas você não deve usar isso como critério principal para decidir qual implementação usar. Código menor não é necessariamente "melhor", tem que usar o que faz mais sentido em cada caso (e entendo que nesse caso não precisava usar regex, por exemplo). Sem contar que, por mais que eu goste de regex, usá-la nem sempre é a melhor solução, além de gerar um overhead muitas vezes desnecessário.
Uma coisa que dá para melhorar são os nomes dos métodos verificaDigitos
e dv
, mas estou sem criatividade no momento (talvez calculaEValidaDigitosVerificadores
e calculaDigitoVerificador
, respectivamente? Não sei).
Se quiser continuar com a regex (e não mudar tanto o restante)
Se quiser muito continuar usando regex, uma pequena melhoria seria criá-las separadamente, já que cada chamada de String::matches
compila a regex novamente. Mas como elas serão usadas várias vezes (toda vez que criar uma nova instância) e não mudam, então uma alternativa seria compilá-las apenas uma vez:
private static final Pattern REGEX_9_DIGITOS = Pattern.compile("^\\d{9}$");
private static final Pattern REGEX_11_DIGITOS = Pattern.compile("^\\d{11}$");
private static boolean ehCadeiaDeNoveDigitosNaoIniciadaEmZero(String cadeia) {
return REGEX_9_DIGITOS.matcher(cadeia).matches();
}
private static boolean ehCadeiaDeOnzeDigitos(String cadeia) {
return REGEX_11_DIGITOS.matcher(cadeia).matches();
}
Ou:
private static final Matcher REGEX_9_DIGITOS = Pattern.compile("^\\d{9}$").matcher("");
private static final Matcher REGEX_11_DIGITOS = Pattern.compile("^\\d{11}$").matcher("");
private static boolean ehCadeiaDeNoveDigitosNaoIniciadaEmZero(String cadeia) {
return REGEX_9_DIGITOS.reset(cadeia).matches();
}
private static boolean ehCadeiaDeOnzeDigitos(String cadeia) {
return REGEX_11_DIGITOS.reset(cadeia).matches();
}
A diferença é que a primeira opção com Pattern
é thread-safe, enquanto a segunda (com Matcher
) não. Sem saber qual o cenário em que será usada a classe, não dá para sugerir qual o "melhor".
Detalhe que existem CPF's que começam com zero, e não considerar esses casos pode dar problemas no mundo real. Por isso mudei suas regex, que não deixavam o CPF começar com zero. Agora elas só verificam se tem 9 ou 11 dígitos, sejam eles quais forem.
Os métodos ehCadeiaEtc
não precisam verificar se a String
é null
, pois você só chama esses métodos em pontos onde já é garantido que não é.
Tem outro detalhe "estranho", neste if
dentro do construtor:
else if (ehCadeiaDeOnzeDigitos(cpf) && isValido(cpf))
É estranho porque dentro do método isValido
temos:
private static boolean isValido(String cpf) {
return ehCadeiaDeOnzeDigitos(cpf) && cpf.substring(NRO_DIGITOS_SEM_DV, NRO_DIGITOS_COM_DV).equals(moduloOnze(cpf.substring(0, NRO_DIGITOS_SEM_DV)));
}
Ou seja, ehCadeiaDeOnzeDigitos(cpf)
é chamado duas vezes, desnecessariamente. Decida onde é melhor chamá-lo (no if
ou dentro de isValido
) e faça-o somente uma vez. Decida também se vai usar o prefixo eh
ou is
(tanto faz, desde que seja consistente em todo o código).
E dá para ficar elucubrando eternamente sobre várias coisas: as constantes NRO_DIGITOS_etc
são realmente necessárias? Se forem, precisam ser públicas? Os métodos private
também precisam ser static
? (funcionalmente, não faz diferença, já que nenhum deles altera ou acessa o estado da instância) A mesma classe que representa o CPF deveria ser responsável pela formatação e criação de uma instância aleatória? E por aí vai... (e sem requisitos reais, fica difícil decidir ou opinar algo - mesmo com requisitos já seria uma discussão boa)
is
eeh
, eu usariaé
:P Eu tenho um treco quando vejo umfalse ==
. Mas você não usou RegEx pro cálculo que era o que estava sendo discutido no chat. Usou pra validação, aí até faz algum sentido, ainda que eu não usaria (eu faria em uma linha mais rápido e na minha opinião mais legível), mas não está estritamente errado usar. Fica a questão se precisa fazer essa validação isolada, se não precisa, aí está usando algo desnecessário. Huum, acabei de ver, tá tudo privado, então acho que é exagerado.