I am creating a CSV consumer (with Java). There is one field / column that should contain one of the values "Rename" or "Move".
I implemented this by allowing mixed case of letters, and blank space at either end. To be precise I used myString.trim().toUpperCase() on the parsed string.
This makes my code less brittle and more robust, but seems to ignore the "fail fast" strategy.
Should I leave the code as it is, or avoid .trim().toUpperCase()?
If the values in the CSV does not exactly conform to the agreed-upon format, then you should fail fast and investigate the issue. It might be an error in the specification or a bug in the software on either end, but in any case you want to investigate it immediately. You do not want to process illegal data and potentially gloss over bugs or data corruption.
There are plenty of cases where the meaning of words may differ due to case - the classic example is "Turkey" (a country) and "turkey" (a large bird). In your particular case with just two values it might seem reasonably safe, but it is still an unnecessary risk which provide no benefit, and as a general rule it is a very bad idea.
There might also be other systems which process the same CSV file but treat case-discrepancies different, leading to inconsistent transactions. You absolutely want to investigate such issues immediately rather than sweep them under the rug.
Of course, if it is specifically agreed that the format is case-insensitive then converting to upper case is fine, but this would not be a question of robustness, just of following the spec. (Although case insensitivity is a can of worms if you go outside the ASCII character range, so it is not necessarily a good default.)
The question of white-space is pretty cut-and-dried. The RFC for CSV states that "Spaces are considered part of a field and should not be ignored." So unless you have specifically agreed with the producer of the CSV that whitespace should be ignored, you should not trim fields.
If you have to do with end-user input (e.g. someone typing into a search box) it might be a good idea to trim, perform case-insensitive matching, ignore diacritical, accepts a certain degree of misspellings etc. But this is a completely different scenario than machine-to-machine communication like a CSV.
Fail fast means you fail at the earliest time you spot a mistake, so you can go and check what is wrong, and not work with wrong data until it is really bad.
In this case, what was the definition of the data? Was it defined that it can be only "Move" or "Rename"? If so, then fail on "MOVE" or "rename" and take the failure and go to your data source and ask them.
Maybe the data is wrong, then it is good you caught it early. Before they send over 5000 files or before you process the data in the other fields, that may have the wrong capitalization, too.
Maybe the definition is wrong and it should have read
"move" or "rename" in any capitalization
Then fix it in the definition.
Either way, right now, when you fail fast, it is an easy fix. Either the producer produces one new file or you ammend the definition slightly. No harm done. Don't wait to ask this question until numerous files have already been processed. That is the beauty of "fail fast".
For situations where some data constellation indicates a bug in code, "fail fast" is usually the only viable a strategy - the program shall be stopped, and the programmer or vendor needs to fix the bug. For processing input data from an external source, "failing fast" can be a viable strategy, too, but here it can also be viable to make the process failure-tolerant to some degree. It not always sensible to be as strict about such input data as about one's own code.
So the important two questions you need to ask here are:
Where does the csv file come from? and,
How strictly / formal is the spec for the csv about the attribute in stake?
In case it comes from an external source not under my direct control, I would not stop processing when a column's content doesn't 100% conform to a spec, but can still be identified reliably. It may be debatable if it is a good idea to apply Postel's law, but at least for data which may have been entered by a human, or in cases where specs leave some room for interpretation, it can be best to be somewhat liberal in regards to extra spaces and upper/lower case.
In case the csv is output of another component under my direct control and part of the same program I am currently writing, and wrong letters or additional spaces are indicators for potential error in that component, I would consider "fail fast" instead and stop processing immediately. Same holds when when the spec for the CSV file is strictly excluding other spellings and extra spaces.
One could also try to log any variance against a spec, but that will only make sense when there is a chance the "external source" (for example, the maintainers of an external component producing that csv file) will ever get the logging results.
In the end, it is a case-specific question, a trade-off between
how much costs will it cause to accept potentially wrong data, vs.
how much costs will it cause to refuse some most probably right data?
This is a data normalization problem.
I know, it's tempting to just throw myString.trim().toUpperCase() in wherever it's needed. It's a quick and simple change. But once it's made, it's impossible to unmake it. So don't do it unless you are rock solid sure.
Rather, make this a separate step. Something that has to be done to the data before your program gets to it. That way it's still possible to use the data and your program. But your expectations are clear.
I know, it means you have to actually decide if uppercase is going to be your standard rather than just accepting whatever. But if you've ever been in a codebase that has a mix of tabs and spaces together then you know how awful it is when no one sets a standard.
Please, stop doing this to us. It hurts.
Relax specification last!
Initial specification and reference implementation should be as strict as possible (there should be only one way to convey a given dataset). Permissive specification would hinder your format after a few years
The fact that you are asking this question indicates, that there is no well-defined specification for the file format and you are likely writing the reference implementation.
It is very important for initial specification and reference implementation to be as restrictive as possible, because relaxing the requirements is irreversible. Permissive parser would allow alternative interpretations for the specification, splitting the community and making it hard to introduce changes (extensible specification is fine, permissive is not).
While a permissive specification may help a private project and would be very convenient in a startup, it will not last.
There are plenty of horror stories about JSON and HTML which prove this point, but those are large and and widely used. My personal experience confirms, that the same principle applies to medium-sized projects due to versioning and backward compatibility problems. Do not rest easy, thinking that your smallish web-site or desktop application are not affected!
If the format to parse is error-prone or inconvenient to use, before trying to relax the requirements, consider adding a good error reporting and provide a validation tool. It is trivial to produce a CSV for however strict requirements, as long as bugs are easily detected and reproduced.
When your format is eventually adopted (either by a wide audience, or multiple product versions or multiple products) you may find incompatibilities in the ecosystem. Do not relax the specification! First, add a warning system in your reference implementation, that accepts invalid input, but reports an error. Be careful when implementing it, make sure that relaxed implementation does not interpret valid input differently.
Do not attempt to make the protocol extensible by relying on permissive parser. Provide strict rules for extensions. Plan for extensions from the start. Remember YAGNI - if you really think that no extensions are needed, it is fine to leave protocol fixed. CSV is extensible if header line is required and column names are well-defined.
Consider a versioning scheme for your data. Versioning is paramount to remove or change a column. Sometimes a well-defined file extension is all it takes. A defined file extension is useful even without versioning, as it further restricts your initial specification (which is a good thing).
There is nothing bad with a permissive protocol that is:
Once the reference implementation is stabilized and receives user feedback, the extension can be carefully considered and more inputs can be considered valid.
Yes, you are right. Lets consider some malformed csv though:
one, two, three, Move, five //correct csv pass - should pass
one, two, three, Move five //missing comma fail - should fail
one, two, three, mo ve //missing comma and split value fail (extra space in middle not trimmed)- should fail
I'm really having to struggle to find cases where trim().toUpper() would cause a pass when you would want a fail.
You would have to introduce some extra critera, such as Move being different from mOve for this case of not failing fast to actually cause an issue.
But if we imagine the more general case you have introduced extra logic which is not part of the requirements and this is always introducing a risk of edge case errors.
In fact I have a good(?) real life example
We have a linter that checks for some best practice in our code base, its been internally designed to check for the version of .net being used. I had a project where I had put net8 as a target framework. This is technically incorrect, I should use net8.0 but vs2022 doesn't care and it works. However the internal linter tool fails as its been programmed to match the spec exactly.
If vs2022 had "failed fast" on my incorrect value I wouldn't have been left with a weird "what the hell is going wrong here, everything looks right" bug to fix.
If you change the first step in your process to accept a value outside of the spec, then you could introduce similar issues on downstream components which look back at the source csv for some reason.
trim(). Only the line one,two,three,Move,five would match the specification.
Commented
Jul 13 at 14:40
“Fail fast” is better than “hide failures until the worst possible time” but best is “don’t fail”.
But please decide: If you expect “whatever”, is “ WHATevER “ an acceptable input to you? If yes, use trim + toUpperString and compare to “WHATEVER” and avoid an incorrect failure. If you think that only “whatever” is acceptable then check for this. (In that case, trim + toUpperString would be daft if you spend extra work in order to not fail fast, why would you do that? No check at all would be avoiding “fail fast”).
I suggest you look into regular expressions. This can help you create a likely cleaner and more flexible solution that still performs quite well.
Failing fast is meaningless if the fail case is rare, like with an improperly formatted input. Then there is no gain in failing fast.
Fail fast in my mind applies to the way you approach a complex problem. You identify the hard parts and focus on those first. If you fail to tackle those, you lost the minimum amount of time. If you succeed, the rest will be smooth sailing.
