CodeSOD: Internal Validation

If you're doing anything financial in Brazil, you have to manage "CNPJ" numbers. These numbers are unique identifiers for every business and every branch of that business, along with a pair of check-digits to validate that it's a correct code. Everyone from banks to accountants to businesses needs to track and manage these.

When Patria joined a microscopic startup as an intern. The startup made an accounting package, and thus needed to track CNPJs. She asked the lead developer, "Hey, how do I validate those check-digits?"

"Actually, as an intern, that's probably a really good beginner task for you."

They were a very new startup, so it wasn't a surprise that the current validation routine was simply return True. Patria, with no industry experience, had her task, and ran off to solve the problem.

"Could you review my code before I merge?" she asked the lead dev.

"Nah, just go ahead and merge, I'm sure it's fine."

This is what Patria merged.

public static bool ValCNPJ(string cnpj){ string cnpj_a = Util.fillWith(Util.trimZero(cnpj.Trim()), 14, "0"); int[] cnpj_n; cnpj_n = new int[14]; int um, dois, tres, quatro, cinco, seis, sete, oito, nove, dez, onze, doze, treze, soma, dv1 = -1, dv2; Boolean cnpj_v = true; //Checando tamanho do numero if (cnpj_a.Length != 14) { cnpj_v = false; } else { for (int i = 0; i < 14; i++) cnpj_n[i] = Convert.ToInt32(cnpj_a.Substring(i, 1)); //Checando primeiro digito verificador um = cnpj_n[0] * 5; dois = cnpj_n[1] * 4; tres = cnpj_n[2] * 3; quatro = cnpj_n[3] * 2; cinco = cnpj_n[4] * 9; seis = cnpj_n[5] * 8; sete = cnpj_n[6] * 7; oito = cnpj_n[7] * 6; nove = cnpj_n[8] * 5; dez = cnpj_n[9] * 4; onze = cnpj_n[10] * 3; doze = cnpj_n[11] * 2; soma = (um + dois + tres + quatro + cinco + seis + sete + oito + nove + dez + onze + doze) % 11; if (soma < 2) { if (cnpj_n[12] != 0) cnpj_v = false; else dv1 = 0; } else { dv1 = 11 - soma; if (cnpj_n[12] != dv1) cnpj_v = false; } // Checagem do segundo digito verificador um = cnpj_n[0] * 6; dois = cnpj_n[1] * 5; tres = cnpj_n[2] * 4; quatro = cnpj_n[3] * 3; cinco = cnpj_n[4] * 2; seis = cnpj_n[5] * 9; sete = cnpj_n[6] * 8; oito = cnpj_n[7] * 7; nove = cnpj_n[8] * 6; dez = cnpj_n[9] * 5; onze = cnpj_n[10] * 4; doze = cnpj_n[11] * 3; treze = dv1 * 2; soma = (um + dois + tres + quatro + cinco + seis + sete + oito + nove + dez + onze + doze + treze) % 11; if (soma < 2) { if (cnpj_n[13] != 0) cnpj_v = false; } else { dv2 = 11 - soma; if (cnpj_n[13] != dv2) cnpj_v = false; } } return cnpj_v;}

The underlying goal is to do a checksum with a mod-11 rule. If you aren't comfortable with for-loops, and only passingly comfortable with arrays of integers, it looks pretty good. Patria writes, "It's not The Brilliant Paula Bean and it does actually work as it should, but dear Lord do I look at this snippet of code and cringe."

This is definitely bad code, but in its badness, I actually appreciate how clear it makes the rules for applying the checksum. Well, mostly clear, some of those conditionals get a little ugly, and nobody likes it when you if (someCondition) foo=False.

This story has a much happier ending than usual. Patria is now the lead developer at the same company. She's happily thrown away this code, replaced it with something much cleaner, and always helps interns and juniors with scrupulous and constructive code reviews.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!