6 ms·
Over the years I have realized that some comments are needed and useful. These days I add comments when there is something particularly tricky, either
by JohnBooty 3y ago
Over the years I have realized that some comments are
needed and useful. These days I add comments when there
is something particularly tricky, either with the
implementation, or in the domain.
Ah, yes. Another convert. There are dozens of us. Dozens!!
Nothing makes me feel crazier than trying to get fellow engineers to comment their code. Code alone can only tell you the "what." If the "why" is not obvious, comment it.
Unit testing private methods.
I am always dismayed by engineers who vote a hard and unyielding "no" on this issue.
I guess ideally your private methods should not be tested directly. If you work at an ideal shop doing ideal things under ideal conditions, please let me know if you are hiring.
In all or most of these cases the tests for private code will hopefully be somewhat temporary; perhaps think of them as scaffolding used during construction or renovation.
- For example, perhaps you have a good reason to write the private methods first and you would like to make sure they are sound before proceeding.
- Perhaps you have a division of labor due to a time crunch and you are writing the private methods while somebody simultaneously writes the public methods.
- Perhaps you are encountering some thorny preexisting code with no test coverage and you would like to just make sure things work.
- Perhaps the public methods are undergoing a lot of flux and you would like to make sure the private methods do not suffer regressions during this flux
- Sometimes it's just easier to test private methods directly rather than indirectly via a public interface. Maybe this is a code smell, but also maybe you don't have time for a full refactor.
- mkoryak 3y agoIf you don't have time to write good tests, maybe consider not writing any tests. I've worked for many a startup where we didn't have any tests because we rewrote everything every few months. And that was ok because everyone was happy to have a lot of shit written fast. I guess maybe what I'm trying to say is that shitty tests are sometimes worse than no tests.
- JohnBooty 3y agoI don't agree that private method tests are shitty. However, I do agree that maybe there is a time during which it's OK to just not have tests. If you are just spitballing it, prototyping, code jamming, maybe even blasting out an MVP for alpha or beta testers. Sure. Part of mastering a craft is making reasoned decisions about breaking rules.
- sverhagen 3y agoI can see a process of triaging and deciding that some code isn't worth testing (yet), but the broader idea of "this is just a prototype", I'd like to push back on, unless you work in the rare place where they actually start over after the prototype. "Blasting out an MVP," you say? That should be production code then, with production-grade testing. I too work for a startup, and well-tested code allows us to release often and with confidence. I am humbled every day by unit tests slapping my wrist when I attempt something stupid.
- josephg 3y ago> unless you work in the rare place where they actually start over after the prototype. Every line of code will last some amount of time. Some lines will be maintained in production for decades. And others will be thrown out in a few weeks. For example, if I’m mocking up a UI, the css I write won’t outlast the mock-up. Or doing a game jam. Or prototyping a different way to structure some code. Unit tests are useful for long lived code but they slow down your capacity to do scrappy iterations. Whether you need to prioritise short term velocity or long term reliability depends on the needs of the project you’re working on.
- sverhagen 3y agoCode just tends to live longer than developers often anticipate. They put more trust in their organizations than is warranted about how much time they will be given later to go back and clean things up. (This probably happens because the original work is over-estimated and then the project is delivered under in a crunch, instead of having some breathing time to clean things up.) But again, this is all situational, you may work in different circumstances than I ever have.
- sanderjd 3y agoI've actually become a bigger fan of unit testing private methods over time. :shrug:
- Groxx 3y agoYeah - the alternative is often to make your non-test code worse to make it more testable. Test private methods when you have sufficient behavior in them that it's worth testing in isolation.
- appplication 3y agoThis is all there is to it. The dogma around not testing private methods is entirely unconvincing. I have some very complex private methods that it’s just easier to write some thorough unit tests for, and it guarantees the contract my public methods rely on. I really do not see why that is controversial.
- naruhodo 3y agoI have given up on being annoyed or proselytising. It's much less stressful. I write the documentation that I would like to see myself.
- sverhagen 3y agoIf your class is doing so many things that you can't reasonably test everything but through the private methods, how about extracting a helper for those things which has testable public (or package private, if you have to) methods!? Did I just sneakily bypass the rules, or make the code better overall? I don't know anymore...
- BugsJustFindMe 3y ago> If your class is doing so many things that you can't reasonably test everything but through the private methods, how about extracting a helper for those things which has testable public When your solution to not wanting to write tests for important behavior is to trick yourself into wanting to by adding extra layers of code for no other reason, maybe the rule stopping you from doing it was wrong.
- sverhagen 3y agoI acknowledged right away that it was a hack. And if it wasn't clear, by that I meant that the private method rule shouldn't be hard. It's can still be a smell, though, but I'm trying to be pragmatic.
- boxed 3y agoIt's not about that imo. It's about having tests that are limited in scope as much as possible, so if they fail it's much easier to understand the problem. Ideally tests should be run in a specific order, from most low level to most high level. So tiny basic functions first, then functions that use those functions, then functions that use THOSE functions, etc until you have end-to-end tests. Unfortunately I know of no testing system that has such a hierarchy.
- The_Colonel 3y agoBasically every decent test framework has a concept of test suites which can be used for this purpose.
- 3y ago
- oweiler 3y agoNo one has ever said that you shouldn't comment your code. "Comment the how, not the why."
- diarrhea 3y agoA great deal of people have. Uncle Bob and his substantial following, for example.
- livrem 3y agoMany of his followers perhaps, but at least in the Clean Code book there is a long chapter on comments, where about half is dedicated to "Good Comments" that include many reasons for when and how to comment code. Yes, it has a section titled "Comments Do Not Make Up for Bad Code"... but he also writes about how there "are certainly times when code makes a poor vehicle for explanation" and that some comments are "necessary or beneficial" like "Informative Comments" or "Explanation of Intent" etc. That chapter is one of the few I remember as somewhat useful in the book (I am not much of a follower or fan... but I did enjoy his Clean Architecture book).
- rustybolt 3y agoI have heard many people say this.
- jader201 3y agoI don’t think you shouldn’t comment your code, but in my experience, good code doesn’t need much, if any, comments. This is because I define “good code” as simple code. If code gets to the point that it needs to be explained with comments, then there’s a good chance there is opportunity for simplification. In other words, if I can’t read code and follow what it’s doing — or why it’s doing it — then I won’t ask them to add comments, but to instead refactor/simplify the code. (This is assuming the code can’t be made more readable with simple name changes, which a lot of times is all that is needed.) Yes, there are times when the “why”, even with simple code, needs to be explained. Comments in this case are fine. But ideally this is the exception and not the norm.
- Yodel0914 3y agoI'm still firmly in the "no" camp on testing private methods. By definition, anything your class does in the real world is done via its public interface; I'm not sure why I need to care about what's done under the covers. I've actually moved further and further away from unit testing over the years (after being a pretty big TDD fan for a long time). In terms of bang for buck, integration tests across your public API are the best IMO. You're testing how your API is actually used. The problem historically has been that they're fragile to refactoring and difficult to run, but with the right tooling you can get around that.
- turtles3 3y agoI find the opposite wrt. refactoring - that is to say integration tests are _more_ robust to refactoring than unit tests, and that's one of the reasons I strongly prefer them. Unit tests are deeply coupled to the internal structure of your system - refactoring often implies changing unit tests, which opens the door to bugs where the code and test both change to match each other. As you say, integration tests validate your public API, which from a 'correctness' point of view is really the only thing you care about, not the internal structure of the system. That's why I love integration tests, you can make sweeping refractors without needing to change the tests, because the test will still tell you whether the behaviour of the whole system is correct.
- throwbadubadu 3y agoYes, especially for some unit testing is done like "atomic testing" (test the smallest possible part, if they could they would test individual lines), but turns out that the putting together is another crucial part with often not less complexity and where the corner cases come in. In that sense even not much like the unit vs integration testing difference & debates, because that line is blurry and just depends on your definition of an unit: It may be the single heavy algorithmic function, it may be the whole state machine, it may even be two heavily dependent and cooperating tasks in an embedded RTOS... and you always know you are in trouble if people discuss more what an unit test is and not is (instead of the value whatever thing provides). I "love" btw those code bases rich with tests but 99% of the time not finding a bug, but breaking on the tiniest detail change.. not supporting refactoring like it should be but making it slow and everyone wanting not to refactor because of the crap test rat tail.
- thom 3y agoJust to intensify the sacrilege here: I’ve been happy testing private behaviour with tests directly alongside the code. This way you can easily sweep away the test code if you change the implementation, and it serves as decent documentation for complex implementation details. The tests obviously disappear in production builds either way.
- JohnBooty 3y agoI’ve been happy testing private behaviour with tests directly alongside the code. I'm intrigued but I don't understand. What does this look like? What language is this?
- thom 3y agoIn Clojure, for example, tests are defined by a macro that decorates a function with certain metadata marking it as a unit test. So they can live and be run from anywhere, but get stripped out of proper builds. You can also go one further, and wrap a function definition in a `with-test` block, so any assertions you make against the function are kept directly alongside its definition, which is a nice form of documentation. What this means is that you can have tests for implementation details, that are cordoned off from your main test suite (which defines your code's real requirements and public API). The benefit is that if you change implementations and suddenly a function is no longer needed, you can just delete the test code right there in the same file, so refactoring is very slightly easier and more obvious. You can also decide whether they should be run during your build process or just at dev time.
- JohnBooty 3y agoThat's interesting, thank you!
- SenAnder 3y agoWhy you should test private methods: To make sure they work correctly. This may be significantly easier than testing the public methods that use them. Why you shouldn't test private methods: Because the implementation might change, obsoleting the tests. But everything might change (except the end-user requirements, haha), so by this argument you shouldn't test anything except what is in the spec/what the user will see. The only benefit of not testing private methods, is the same benefit that not testing anything else brings.
- moring 3y ago> Unit testing private methods. Adding the single biggest reason I have encountered: Because the overall functionality is too complex to test through the API alone, so the alternative is to extract part of the "private" functionality to a separate class that would then be public itself, making functionality public that shouldn't be. By that, the discussion about "testing private methods" ignores the elephant in the room, which is that the language used doesn't have a rich concept of public vs. private in the first place. As evidence for that I'd like to mention that I never had that problem in Rust, because I can simply extract the private functionality into helper functions within a private sub-module and unit-test them there, without that functionality becoming public to the rest of the code.
- mcv 3y ago> Code alone can only tell you the "what." If the "why" is not obvious, comment it. But isn't that the common view? I think most programmers would agree that the "why" deserves documenting. The problem is that in the heat of the moment, most forget.