> it was only supposed to pre-pend one or two zeros when importing zip codes from CSV files. Anything with fewer than three digits is supposed to be an invalid code
The true problem here lies with the poor choice of function name on the part of the original author and lack of commentary. The name "formatted_zip_code" automatically primes people to expect that you passed a (possibly mal-formed) zip code and the function should format it properly. Under that metric, the original code was indeed deficient and should have been changed. Whether it retained the original style is secondary, but there were clear functional deficiencies.
"Anything with fewer than three digits is supposed to be an invalid code" is not reflected in the code (why does it return digits and not trigger an error there?) and apparently there are no comments, so it should be no surprise that it needed to be fixed.
Exactly. The fault lies with the original author in not explaining the intent behind the function, why it is written in the way it is.
I've seen this kind of code time and again from coworkers who have the "code should be self-documenting" mentality, who refuse to put in comments explaining why they wrote a function a particular way. But what they think is self-documenting, very, very, very rarely is to others.
Problem is that specs change and need tweaks afterwards. I wouldn't always think to update a method name, though I am quite liberal with comments. I guess I should start to look out for that as a way of improving my code.
def formatted_zip_code(digits)
case digits.size
when 4 then "0#{digits}"
when 3 then "00#{digits}"
# 2 or less digits is an invalid input
else digits
end
end
-> if the argument doesn't conform to contract, an assert/exception should be thrown so that the caller can recognize the existence of an incorrectly used function.
Are there zip codes that are 2 digits? I'd fall fallacy to this post and once again argue to the contrary. If this function is about formatting zip codes with leading zeros, and zip codes are either 4 or 3 digits. Why would an invalid zip code be passed to this method, a formatter?
Normally I'd agree with your argument about failing to meet a contract, but why would this formatter method get anything but a valid zip code.
The first 3 digits of a zip code represent the sectional center facility code, the smallest of which is 005. One would assume that 00501 is the smallest zip code (which apparently maps to HOLTSVILLE NY )
> why would this formatter method get anything but a valid zip code.
Ask not, lest ye receive an answer.
Seriously speaking, you can't trust that callers will be sane unless you establish a priori with a higher level contract that all possible callers will be sane.
The true problem here lies with the poor choice of function name on the part of the original author and lack of commentary. The name "formatted_zip_code" automatically primes people to expect that you passed a (possibly mal-formed) zip code and the function should format it properly. Under that metric, the original code was indeed deficient and should have been changed. Whether it retained the original style is secondary, but there were clear functional deficiencies.
"Anything with fewer than three digits is supposed to be an invalid code" is not reflected in the code (why does it return digits and not trigger an error there?) and apparently there are no comments, so it should be no surprise that it needed to be fixed.