4 ms·

The trouble comes when the others working on your code are crazy enough that that strange code often is useless, so you have to spend the time trying to work ou

by dri_ft 8y ago

The trouble comes when the others working on your code are crazy enough that that strange code often is useless, so you have to spend the time trying to work out whether it's tear-out-worthy.

javajosh 8y ago
A sign of maturity is to a) recognize when code is crazy, and b) document why it is so in a comment.
- couchand 8y ago
  Here let me fix that for you: "document why it is so in an executable test".
  - pure-awesome 8y ago
    Sometimes possible, but not always. An executable test is good for documenting what a piece of code should do, but not always entirely sufficient for documenting why.
    - jsight 8y ago
      Exactly, and in the pathological case it ends up documenting the why incorrectly and the code is still wrong.
      - pure-awesome 8y ago
        Yes, but this is hardly an argument against using comments. There will always be a pathological case. The question is what approach will increase your chances of success, what's least likely to lead to problems, what's the cost-risk-benefit tradeoff etc. Comments detailing the intent of a non-obvious function are valuable for maintainers wanting to figure out what's happening in the code. They are written in human-oriented business-oriented language, so they don't take effort to read. The intent of the code should not change that often. If they diverge from the code, the effort of changing them should be small enough to be worth it. Ideally they should be treated as important and if they are wrong it should be picked up in the code-review. Comments detailing just the description of what a function does, by contrast, are not valuable, since that information can be deduced from the code itself. Every time the function changes, the comment will change. Reading them is as much effort as reading the code, so if they diverge it won't be picked up as easily. There are always exceptions to these kinds of guidelines, of course, but this at least gives motivation for why I think comments should be used in this way.
    - couchand 8y ago
      Though I agree that it's not always possible, I would contest your second point, at least for the majority of testing effort. Sure, a unit test might just document what code is doing, because that's really the only why. But most testing effort should go into describing the why more carefully, which must be done at a higher level, as close as possible to how a user will interact with the thing. Writing tests at the user's level and not getting bogged down in minutia requires careful factoring of the what and why. The what changes frequently, so if most of your tests are overly focused on the what, you will constantly need to update them as the code changes. The why changes much more slowly, so if your tests are written to exercise the why rather than the how, they will be resilient to changes in the implementation. Of course, the what has to be somewhere, and when it changes there will need to be corresponding test updates, but these shouldn't require updating each individual test. The what can be kept cleanly factored out into helpers, leaving the why to remain as the essence of each test.
  - javajosh 8y ago
    It's not always possible (or even advisable) to document with a test; to take two trivial examples from recent memory, one was a wrapping bug in IE 11 and the other was a typing issue with TypeScript. These issues required comments, not tests. (Granted, for the IE11 bug the ideal test harness would include a Win/IE virtual machine in a known state along with a good robot control and the ability to check screen geometry, but as of 2019 that's science fiction)
    - couchand 8y ago
      Your second example is a great example of how sometimes it's just not worth it. You're probably right that it makes sense to just write a comment and move along (after you've replicated the bug and filed an issue against the TypeScript compiler). Eventually it will get fixed, your team will upgrade their system, and you can remove the workaround. I'd argue the first example illustrates my point quite well (but forgive me if this gets too ivory tower). It would seem highly unlikely that the wrapping bug is intimately coupled with whatever else the component is doing. It will likely come up again in some other situation, in this project or another. After all, some users will be on this browser forever. All this suggests we would like to factor it out, in the name of the single responsibility principle and don't repeat yourself. And the extracted component should then be thoroughly tested to it's own why. That component's why, in particular, is to work around the browser bug, so a test specification that describes precisely which versions of Windows and IE are required to be worked around is actually exactly what you want. As to good robot control and screen geometry, I don't think that's really science fiction anymore, unless your requirements are more exacting than mine.

by dri_ft 8y ago

The trouble comes when the others working on your code are crazy enough that that strange code often is useless, so you have to spend the time trying to work out whether it's tear-out-worthy.

javajosh 8y ago

A sign of maturity is to a) recognize when code is crazy, and b) document why it is so in a comment.

couchand 8y ago
Here let me fix that for you: "document why it is so in an executable test".
- pure-awesome 8y ago
  Sometimes possible, but not always. An executable test is good for documenting what a piece of code should do, but not always entirely sufficient for documenting why.
  - jsight 8y ago
    Exactly, and in the pathological case it ends up documenting the why incorrectly and the code is still wrong.
    - pure-awesome 8y ago
      Yes, but this is hardly an argument against using comments. There will always be a pathological case. The question is what approach will increase your chances of success, what's least likely to lead to problems, what's the cost-risk-benefit tradeoff etc. Comments detailing the intent of a non-obvious function are valuable for maintainers wanting to figure out what's happening in the code. They are written in human-oriented business-oriented language, so they don't take effort to read. The intent of the code should not change that often. If they diverge from the code, the effort of changing them should be small enough to be worth it. Ideally they should be treated as important and if they are wrong it should be picked up in the code-review. Comments detailing just the description of what a function does, by contrast, are not valuable, since that information can be deduced from the code itself. Every time the function changes, the comment will change. Reading them is as much effort as reading the code, so if they diverge it won't be picked up as easily. There are always exceptions to these kinds of guidelines, of course, but this at least gives motivation for why I think comments should be used in this way.
  - couchand 8y ago
    Though I agree that it's not always possible, I would contest your second point, at least for the majority of testing effort. Sure, a unit test might just document what code is doing, because that's really the only why. But most testing effort should go into describing the why more carefully, which must be done at a higher level, as close as possible to how a user will interact with the thing. Writing tests at the user's level and not getting bogged down in minutia requires careful factoring of the what and why. The what changes frequently, so if most of your tests are overly focused on the what, you will constantly need to update them as the code changes. The why changes much more slowly, so if your tests are written to exercise the why rather than the how, they will be resilient to changes in the implementation. Of course, the what has to be somewhere, and when it changes there will need to be corresponding test updates, but these shouldn't require updating each individual test. The what can be kept cleanly factored out into helpers, leaving the why to remain as the essence of each test.
- javajosh 8y ago
  It's not always possible (or even advisable) to document with a test; to take two trivial examples from recent memory, one was a wrapping bug in IE 11 and the other was a typing issue with TypeScript. These issues required comments, not tests. (Granted, for the IE11 bug the ideal test harness would include a Win/IE virtual machine in a known state along with a good robot control and the ability to check screen geometry, but as of 2019 that's science fiction)
  - couchand 8y ago
    Your second example is a great example of how sometimes it's just not worth it. You're probably right that it makes sense to just write a comment and move along (after you've replicated the bug and filed an issue against the TypeScript compiler). Eventually it will get fixed, your team will upgrade their system, and you can remove the workaround. I'd argue the first example illustrates my point quite well (but forgive me if this gets too ivory tower). It would seem highly unlikely that the wrapping bug is intimately coupled with whatever else the component is doing. It will likely come up again in some other situation, in this project or another. After all, some users will be on this browser forever. All this suggests we would like to factor it out, in the name of the single responsibility principle and don't repeat yourself. And the extracted component should then be thoroughly tested to it's own why. That component's why, in particular, is to work around the browser bug, so a test specification that describes precisely which versions of Windows and IE are required to be worked around is actually exactly what you want. As to good robot control and screen geometry, I don't think that's really science fiction anymore, unless your requirements are more exacting than mine.