[peheje]

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:

  test("should_match_debit_with_credit_only_if_within_three_day_settlement_window", () => {
      const debit = A_Transaction().withAmount(500.00).asDebit().on(JANUARY_1);
      const creditWithinWindow = A_Transaction().withAmount(500.00).asCredit().on(JANUARY_4);
      const creditOutsideWindow = A_Transaction().withAmount(500.00).asCredit().on(JANUARY_5);
  
      expect(Reconciliation.tryMatch(debit, creditWithinWindow)).isSuccessful();
      expect(Reconciliation.tryMatch(debit, creditOutsideWindow)).hasFailed();
  });

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]

> 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]

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.

[peheje]

I like the idea of using a LegalConstants namespace or Code Owners to signal that we don't own those values.

However, I’d argue that being 'out of our control' is actually the main reason to test them. We treat these as acceptance tests. The goal isn't flexibility, it is safety. If a PR changes a regulated value, the test failure acts as a tripwire. It forces us to confirm with the PM (and the Jira ticket) that the change is intentional before merging. It catches what code structure alone might miss.

[8note]

or very concretely, if legal sets a constant to 0, there should be some test that makes sure the code handles divisions by that number properly, or otherwise fail to deploy.

youd need tests both with the current constant for the current state, and with various bounds on that constant to show that the code always works

[sbuttgereit]

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.

[8note]

with code in good shape, i think i prefer having unnamed tests and instead you read the test to see that its an important function.

however, ive also done code archaeology, and same thing, old inaccurate comments were one of the few remaining things with any idea what the code was supposed to do and got me enough pointers to company external docs to figure out the right stuff.

wiki links, issue links, etc all had been deleted. same with the commit history, and the tests hadnt worked in >5 years and had also been deleted

the best comments on that code were about describing Todos and bugs that existed rather than what the code did do. stream of consciousness comments and jokes

what Ive left for future archaeologists of my code is detailed error messages about what went wrong and what somebody needs to do to fix that error