3 ms·

To the downvoters: why? Genuinely curious.

by crehn 6y ago

Gupie 6y ago
I have not down voted it myself but there is the assumption that unit tests will always catch bad refactorings, that human error doesn't apply to writing unit test, that every developer is perfectly conscientious, competent, is not under tight deadlines etc., that your legacy code base has comprehensive unit tests..
- ziga-miklic 6y ago
  The author here: I agree with @Gupie, having unit tests ensures that you have correctly refactored the code and have not introduced new bugs. Checking the unit tests can also help you better understand what problems the code is solving and the edge cases you may be missing. Normally, if unit tests were missing, I would add them before starting refactoring. In this case, to be honest, I'm not sure how I could have written a unit test to catch the issue. An E2E test is the only one that comes to mind. Please let me know if I am missing an easy way to test for a 404 image when offline. Thank you!
  - alkonaut 6y ago
    The danger is in the combination of all 3: non-obvious assumption (code should work offline), lack of tests, lack of documentation of the assumption. Flip ANY of the three, and there is no problem. Had I been the original author I would have gone for the comment, it’s clearly the simplest one in this case.
NohatCoder 6y ago
Try to actually design the test that would have caught this. There is no way in this universe that one can blindly rely on tests when refactoring.
- alkonaut 6y ago
  Code that is obvious is better. That’s also difficult here (You’d for example make a function call of image_that_should_work_offline(..)” that simply returns its argument, but that’s perhaps a bit contrived). So the second best is simply a comment. // Image inlined because error may be due to dropped connection You don’t have to have tests to prevent bad refactoring. 100% test coverage isn’t realistic for most code bases (or isn’t even a net positive in many cases). self-documenting code, or tests, or comments would have saved the situation. You don’t need to design a test in this case. A one line comment that takes five seconds to write is enough.

To the downvoters: why? Genuinely curious.

by crehn 6y ago

To the downvoters: why? Genuinely curious.

I have not down voted it myself but there is the assumption that unit tests will always catch bad refactorings, that human error doesn't apply to writing unit test, that every developer is perfectly conscientious, competent, is not under tight deadlines etc., that your legacy code base has comprehensive unit tests..

ziga-miklic 6y ago
The author here: I agree with @Gupie, having unit tests ensures that you have correctly refactored the code and have not introduced new bugs. Checking the unit tests can also help you better understand what problems the code is solving and the edge cases you may be missing. Normally, if unit tests were missing, I would add them before starting refactoring. In this case, to be honest, I'm not sure how I could have written a unit test to catch the issue. An E2E test is the only one that comes to mind. Please let me know if I am missing an easy way to test for a 404 image when offline. Thank you!
- alkonaut 6y ago
  The danger is in the combination of all 3: non-obvious assumption (code should work offline), lack of tests, lack of documentation of the assumption. Flip ANY of the three, and there is no problem. Had I been the original author I would have gone for the comment, it’s clearly the simplest one in this case.

NohatCoder 6y ago

Try to actually design the test that would have caught this. There is no way in this universe that one can blindly rely on tests when refactoring.

alkonaut 6y ago
Code that is obvious is better. That’s also difficult here (You’d for example make a function call of image_that_should_work_offline(..)” that simply returns its argument, but that’s perhaps a bit contrived). So the second best is simply a comment. // Image inlined because error may be due to dropped connection You don’t have to have tests to prevent bad refactoring. 100% test coverage isn’t realistic for most code bases (or isn’t even a net positive in many cases). self-documenting code, or tests, or comments would have saved the situation. You don’t need to design a test in this case. A one line comment that takes five seconds to write is enough.