| ▲ | peheje 3 days ago | ||||||||||||||||
I don't necessarily disagree with providing context, but my concern is that comments eventually lie. If the business rule evolves (say the window moves to 5 days) the comment becomes a liability the moment someone updates the code but forgets the prose. The comment also leaves me with more questions: how do you handle multiple identical amounts in that window? I would still have to read the implementation to be sure. I would prefer encoding this in an Uncle Bob style test. It acts as living documentation that cannot get out of sync with the code and explains the why through execution. For example:
This way, the 3 day rule is a hard requirement that fails the build if broken rather than a suggestion in a comment block. | |||||||||||||||||
| ▲ | kmoser 3 days ago | parent | next [-] | ||||||||||||||||
> I don't necessarily disagree with providing context, but my concern is that comments eventually lie. If the business rule evolves (say the window moves to 5 days) the comment becomes a liability the moment someone updates the code but forgets the prose. Yes, comments can drift out of sync with the code. That doesn't mean the comments were a bad idea; it just means the programmer didn't update the comments when they should have. Similarly, even with zero comments, variable names can drift out of sync with their intended purpose, e.g. when they are repurposed to hold values that are different from what their name implies. Again, the solution is to rename the variable so it reflects its purpose (or possibly create another variable). > This way, the 3 day rule is a hard requirement that fails the build if broken rather than a suggestion in a comment block. What happens when a dev changes the test code, but fails to update the string "should_match_debit_with_credit_only_if_within_three_day_settlement_window"? That's effectively no different than comments being out of sync with the code. | |||||||||||||||||
| ▲ | tetha 3 days ago | parent | prev | next [-] | ||||||||||||||||
This however misses an important point: 3 is not in our control. 3 in general is controlled by math-people, and that 3 in particular is probably in the hands of a legal/regulation department. That's a much more important information to highlight. For example, at my last job, we shoved all constants managed by the balancing teams into a static class called BalancingTeam, to make it obvious that these values are not in our control. Tests, if (big-if) written, should revolve around the constants to not be brittle. | |||||||||||||||||
| |||||||||||||||||
| ▲ | sbuttgereit 3 days ago | parent | prev [-] | ||||||||||||||||
Until someone changes the test to be four days, or two, but doesn't update the test name. Ultimately disciplined practice requirements rely on disciplined practices to succeed. You can move the place where the diligence needs to taken, but at the end the idea that comments can lose their meaning isn't that different to other non-functional, communicative elements also being subject to neglect. I don't mean to suggest that a longish test title wouldn't be more likely to be maintained, but even with that long name you are losing some context that is better expressed, person-to-person, using sentences or even paragraphs. I had first hand experiences with this, oddly enough, also working on an accounting system (OK, ERP system... working the accounting bits). I was hired to bring certain deficient/neglected accounting methodologies up to a reasonable standard and implement a specialized inventory tracking capability. But the system was 20 years old and the original people behind the design and development had left the building. I have a pretty strong understanding of accounting and inventory management practices, and ERP norms in general, but there were instances where the what the system was doing didn't make sense, but there was no explanations (i.e. comments) as to why those choices had been taken. The accounting rules written in code were easier to deal with, but when we got to certain record life-cycle decisions, like the life-cycle evolution of credit memo transactions, the domain begins to shift from, "what does this mean from an accounting perspective", where generally accepted rules are likely to apply, to what did the designers of the application have in mind related to the controls of the application. Sure I could see what the code did and could duplicate it, but I couldn't understand it... not without doing a significant amount of detective work and speculation. The software developers that worked for the company that made this software were in the same boat: they had no idea why certain decisions were taken, if those decisions were right or wrong, or the consequences of changes... nor did they care really (part of the reason I was brought in). Even an out-of-date comment, one that didn't reflect the code has it evolved, would still have provided insight into the original intents. I know as well as you that code comments are often neglected as things change and I don't take it for granted that understanding the comments are sufficient for knowing what a piece of code does.... but understanding the mind of the author or last authors does have value and would have helped multiple times during that project. When I see these kinds of discussions I'm always reminded of one of my favorite papers. By Peter Naur, "Programming As Theory Building" (https://pages.cs.wisc.edu/~remzi/Naur.pdf). In my mind, comments that were at least good and right at one time can give me a sense of the theory behind the application, even if they cannot tell me exactly how things work today. | |||||||||||||||||
| |||||||||||||||||