|
Che Delilas posted:Honestly I never know what to do with functions like that, that is, functions that do fairly obvious things. A well-named function and set of parameters should be fairly self-documenting, but I always feel like it's better to have stupid, obvious function header comments than none at all. If for no other reason than that the color of the comment text is a better visual separator than anything else. I see that big block of green, I know I'm at the start of a function no matter how fast I'm scrolling. Also, a number of places auto-generate documentation (for public or internal consumption) based on those comment blocks so having them there may not be optional. Obviously I have no clue whether that's the case for the OP's code base .
|
# ? Oct 12, 2014 13:31 |
|
|
# ? Jun 8, 2024 06:54 |
|
Suspicious Dish posted:The best documentation is: Our code doesn't pass review without these
|
# ? Oct 12, 2014 16:36 |
|
Che Delilas posted:Honestly I never know what to do with functions like that, that is, functions that do fairly obvious things. A well-named function and set of parameters should be fairly self-documenting, but I always feel like it's better to have stupid, obvious function header comments than none at all. If for no other reason than that the color of the comment text is a better visual separator than anything else. I see that big block of green, I know I'm at the start of a function no matter how fast I'm scrolling. I see those as good opportunities to add "See Also" links to things that somebody looking up documentation for an obvious function might actually be wanting.
|
# ? Oct 12, 2014 16:50 |
|
Che Delilas posted:Honestly I never know what to do with functions like that, that is, functions that do fairly obvious things. A well-named function and set of parameters should be fairly self-documenting, but I always feel like it's better to have stupid, obvious function header comments than none at all. If for no other reason than that the color of the comment text is a better visual separator than anything else. I see that big block of green, I know I'm at the start of a function no matter how fast I'm scrolling. Tell me what a ThingInfo is. Is this a low-level method? Do you expect it to be used often? If not, what should I use instead? Do I need to free the ThingInfo after I'm done with it? Better documentation would be: "Gets the ThingInfo for a thing. The ThingInfo is a low-level data structure containing a make, version and body. As the ThingInfo is not updated when the Thing is updated, users should instead use getNameForThing instead. See the ThingInfo documentation for more details."
|
# ? Oct 12, 2014 17:36 |
|
Right - while it may be obvious that the function returns a ThingInfo, it is not immediately obvious why it does so or in which scenarios this can be done. The most important thing is to document why the code exists and what purpose it serves, not so much what it actually does.
|
# ? Oct 12, 2014 18:28 |
|
EssOEss posted:Right - while it may be obvious that the function returns a ThingInfo, it is not immediately obvious why it does so or in which scenarios this can be done. The most important thing is to document why the code exists and what purpose it serves, not so much what it actually does. Yeah, if you document what it actually does, you just end up in a situation down the line where the code has changed, but the commentary hasn't, and they directly contradict each other. Then you get to play the "bug or feature?" game.
|
# ? Oct 12, 2014 18:30 |
|
If you have actually written tests for your code, especially if you've done it in a TDD way, the tests themselves generally serve as documentation for how to use the function and what it does. Also, then you effectively have sample code that can be used if someone else wants to use that method. In theory, anyway. There are still times you want comments, especially of the "this is the purpose of this class" variety. Also, publishing a public API is different, to an extent. But still, a set of tests that demonstrates how the method is supposed to act is far superior than comments above the method that say how it's supposed to act.
|
# ? Oct 12, 2014 18:35 |
|
Suspicious Dish posted:Tell me what a ThingInfo is. Is this a low-level method? Do you expect it to be used often? If not, what should I use instead? Do I need to free the ThingInfo after I'm done with it? What if it's virtual and is meant to be overwritten by a bunch of other classes?
|
# ? Oct 12, 2014 18:59 |
|
Axiem posted:If you have actually written tests for your code, especially if you've done it in a TDD way, the tests themselves generally serve as documentation for how to use the function and what it does. Also, then you effectively have sample code that can be used if someone else wants to use that method. Tests don't look like sample code for someone trying to use a library. If you give me this "just look in the tests/ folder for examples about how to use it" poo poo, I will punch you.
|
# ? Oct 12, 2014 18:59 |
|
QuarkJets posted:What if it's virtual and is meant to be overwritten by a bunch of other classes? I don't know. What if it's virtual and is meant to be overwritten by a bunch of other classes?
|
# ? Oct 12, 2014 19:00 |
|
Suspicious Dish posted:Tests don't look like sample code for someone trying to use a library. If you give me this "just look in the tests/ folder for examples about how to use it" poo poo, I will punch you. Tests: Living documentation of XML comments at the class/method level: Documentation of how and when to use them Regular old in-line comments: Explanation of the rationale behind something weird or hacky New Yorp New Yorp fucked around with this message at 19:57 on Oct 12, 2014 |
# ? Oct 12, 2014 19:04 |
|
Suspicious Dish posted:Tests don't look like sample code for someone trying to use a library. If you give me this "just look in the tests/ folder for examples about how to use it" poo poo, I will punch you. Hence my exceptions for "public API", which was admittedly a poor way of saying "a library". Yes, if you're writing a library, you will need to have documentation above and beyond the tests. (Though you should still make the tests public because they can be useful for documentation purposes). Ithaqua posted:Tests: Living documentation of implementation details Yes, this very much, except that I would say you shouldn't be writing your tests based on the implementation details of a class; you should be writing your tests based around the public methods of the class. Part of the benefit of tests is that you can change the underlying implementation (such as making something more efficient) and be confident that it still works the way it should because your tests pass.
|
# ? Oct 12, 2014 19:33 |
|
Source control is a good place for rationale. "Refactored foo to remove dependency on bar.\n - I know this commit looks weird, but...". Then blame gives you a narrative.
|
# ? Oct 12, 2014 19:38 |
|
Wardende posted:Our code doesn't pass review without these I always use: code:
|
# ? Oct 12, 2014 19:39 |
|
Axiem posted:Yes, this very much, except that I would say you shouldn't be writing your tests based on the implementation details of a class; you should be writing your tests based around the public methods of the class. Part of the benefit of tests is that you can change the underlying implementation (such as making something more efficient) and be confident that it still works the way it should because your tests pass. Fair enough -- pretend I said "intended behavior" instead of "implementation detail".
|
# ? Oct 12, 2014 19:56 |
|
Code is not documentation. It doesn't matter if the code is tests which test other code. Tests need documenting as well.
|
# ? Oct 12, 2014 20:56 |
|
qntm posted:Code is not documentation. It doesn't matter if the code is tests which test other code. Tests need documenting as well. If someone* looking at a method can't understand what it's for and how it's used without referring to external documentation or tests, your code is bad. Sorry. That doesn't mean you have to write a thousand-word essay explaining every trivial getter, but it sure as hell does mean you need to document anything that's non-trivial - and that applies to any code that will ever be maintained, not just public APIs. Unless your employer actively supports paying people to reverse-engineer their own products, I guess, but that seems like a rather inefficient way to run a business. * Someone who knows the language and has a basic understanding of the domain you're working in, at least. For example: you, in six months' time.
|
# ? Oct 12, 2014 22:24 |
|
Soricidus posted:If someone* looking at a method can't understand what it's for and how it's used without referring to external documentation or tests Or worse, asking someone.
|
# ? Oct 13, 2014 00:11 |
|
Soricidus posted:
This is kinda what we do where I work. We have a common set of DLLs for a bunch of different specialized sites, and there's no good way to tell which version of the source matches up with the DLL that is currently in use for a site so we just tend to decompile the DLLs (MSIL) and see what they're doing from there. There's not enough documentation in the source for those DLLs that you're missing anything either, you're kinda stuck trying to figure out what it's doing in each case when you're working on something. ps. this sucks and is a terrible way of doing it
|
# ? Oct 13, 2014 03:42 |
|
carry on then posted:To be clear, it's a very large piece of enterprise software. I honestly don't think the "build the product" stage is that long, but with testing it can take quite a while. Where we are its just over an hour to build our Java app, shame it runs on outdated enterprise server though....
|
# ? Oct 13, 2014 11:23 |
|
Wardende posted:Our code doesn't pass review without these same here plus it checks the header block we have to have on all files, we have had more fails due to missing header than bad code (although a lot is laughable)
|
# ? Oct 13, 2014 11:25 |
|
qntm posted:Code is not documentation. It doesn't matter if the code is tests which test other code. Tests need documenting as well. This. Take the time out of your busy TDD schedule of writing tests for everything, and write me three lines in English describing your function, so I can read that instead of a page of test source code. Don't give me the "well I'll document it if it's a library" line either, if I have to look at your code then I need to know what the hell it does, and nuts to your "my code is self documenting " approach.
|
# ? Oct 13, 2014 13:39 |
|
Volmarias posted:Don't give me the "well I'll document it if it's a library" line either, if I have to look at your code then I need to know what the hell it does, and nuts to your "my code is self documenting " approach. My favorite moment at work was doing a code review for a very smart architect who dogmatically insisted that code be self documenting. Trying for a good five minutes to figure out what a function HE WROTE a month before did. And not even realizing the irony of repeating its self documenting as he was doing so.
|
# ? Oct 13, 2014 13:58 |
|
Volmarias posted:This. Take the time out of your busy TDD schedule of writing tests for everything, and write me three lines in English describing your function, so I can read that instead of a page of test source code. Good tests (i.e. examples of usage) are way better than documentation. The last project I worked on had roughly 100 MB of Office files of design documents and other documentation; guess how many of those were worth reading to actually work on the code.
|
# ? Oct 13, 2014 14:09 |
|
Volmarias posted:This. Take the time out of your busy TDD schedule of writing tests for everything, and write me three lines in English describing your function, so I can read that instead of a page of test source code. I'm working on a Rails API on a team that does TDD and it works pretty well I think. We have almost no comments in our code, but we try to make stuff readable by keeping our functions as short as possible and refactoring anything significant out to another class/function. We really just let the tests describe what the files/methods do. We also use a doc generator that reads the acceptance tests and spits out detail on all the endpoints and example input/output. I'd kill myself if we spent even more time writing comments everywhere since TDD already takes forever.
|
# ? Oct 13, 2014 14:25 |
|
pigdog posted:Okay, I'd give you those three lines. Six months down the line they might be flat out wrong though. Welp, I guess commenting is dead because the comments could be wrong. There's no middle ground here! The path to 100MB of design documents starts with a comment describing the intent of a function! pigdog-PrimeUniverse posted:Okay, I'd give you those tests. Six months down the line they might be flat out wrong though. ErIog fucked around with this message at 17:09 on Oct 13, 2014 |
# ? Oct 13, 2014 17:02 |
|
ErIog posted:Welp, I guess commenting is dead because the comments could be wrong. There's no middle ground here! That's not to say removing comments automatically makes code better, but working on up to date tests is a better time investment than trying to keep comments up to date. pigdog fucked around with this message at 17:33 on Oct 13, 2014 |
# ? Oct 13, 2014 17:31 |
|
pigdog posted:Okay, I'd give you those three lines. Six months down the line they might be flat out wrong though. Or maybe, uh, I don't know, update the comments when you're changing the code?
|
# ? Oct 13, 2014 18:05 |
|
Hey, remember the guy who said architecture diagrams are categorically worthless and you can get all the information you need from reading the code? The absolutism wasn't right then, either.
|
# ? Oct 13, 2014 18:08 |
|
omeg posted:Or maybe, uh, I don't know, update the comments when you're changing the code? I'm not the one bringing about absolutes, just that I've seen it both ways recently, and the guys who rarely write any comments on the basis that naming, SRP and tests should explain the code instead, did produce better code. Personally I'm not that fundamentalist, but given the choice I'd err on the side of self-explanatory code than code that needs comments to explain how it works. pigdog fucked around with this message at 18:29 on Oct 13, 2014 |
# ? Oct 13, 2014 18:20 |
|
The documentation comment about the function isn't supposed to document its internals, only the contract you provide to the caller. And if that contract changes quite often, you built a poor API.
|
# ? Oct 13, 2014 18:42 |
|
Suspicious Dish posted:The documentation comment about the function isn't supposed to document its internals, only the contract you provide to the caller. And if that contract changes quite often, you built a poor API. No doubt documentation, ie Javadoc, on public interfaces that aren't supposed to change is great. The rest of the code behind that facade shouldn't expect the same treatment.
|
# ? Oct 13, 2014 18:57 |
|
pigdog posted:Or you could do something more productive with your time. A refactoring step could change 100's of classes at a time; you could run it, run tests, and be assured that it didn't break anything. That's easier than browsing through those 100's of classes and check if the comments still make sense by manually comparing them to code. Are you seriously saying that taking 10 seconds out of your time to update the comments aren't worth it? You are the person with whom we share code.
|
# ? Oct 13, 2014 19:20 |
|
ratbert90 posted:Are you seriously saying that taking 10 seconds out of your time to update the comments aren't worth it? No, they're not seriously saying that and it's disingenuous for you to cast it as such. There is good tooling for changing large swaths of code in clean, maintainable ways. There are no such tools for changing documentation like that. It's silly to posit that documentation updates would take "10 seconds" when the tooling to do that simply does not exist. We can pick some details for which I'm wrong, and it seems quite popular to do so feel free, but the general point of where tooling is available is not rendered false from such an effort.
|
# ? Oct 13, 2014 19:40 |
|
Could someone who's arguing that tests can make good documentation please post an example of a test suite they'd like to use that way? I'm honestly interested to see what you have in mind.
|
# ? Oct 13, 2014 20:09 |
|
Soricidus posted:Could someone who's arguing that tests can make good documentation please post an example of a test suite they'd like to use that way? I'm honestly interested to see what you have in mind. The reason is fairly simple -- there's an absolute gold standard for what a "good test" is -- it's one that consistently passes on Windows, no matter how weird or hosed up or nonsensible it is. If Windows does it, we need to do it too. Throughout its history the Wine project has learned that MSDN, the documentation for the API functions we're testing, is full of outright lies and nonmentions of bugs, and often applications rely on those bugs working a certain way. In this way Wine's code itself, and its corresponding test suite, can sometimes tell you more about what the Windows API does than MSDN. This is all Wine-specific, of course -- users of the MSDN documentation aren't the people modifying the Windows source code, they're third parties. And Wine developers can't see the MS source code either. And most projects can't have such an easy definition of "good test".
|
# ? Oct 13, 2014 20:58 |
|
ShadowHawk posted:The best argument for TDD is the Wine project. Or at least TDD within the Wine project. See, I'm already sold on the idea that your libraries and APIs should have tests, I really am. What I'm not sold on is the idea that your tests should be your documentation. I've already been in the situation where I had to make test suited to verify third party behavior because their docs were nothing but lies. That didn't stop me from telling them and demanding that they fix their docs along with their behavior. Obviously you can't tell Microsoft to do anything, but you can certainly tell yourself and your teammates.
|
# ? Oct 13, 2014 21:32 |
|
Soricidus posted:Could someone who's arguing that tests can make good documentation please post an example of a test suite they'd like to use that way? I'm honestly interested to see what you have in mind. I regularly jump into the ReactiveCocoa tests to understand how to use various pieces of the framework: https://github.com/ReactiveCocoa/ReactiveCocoa/tree/master/ReactiveCocoaFramework/ReactiveCocoaTests
|
# ? Oct 13, 2014 22:00 |
|
JawnV6 posted:There is good tooling for changing large swaths of code in clean, maintainable ways. There are no such tools for changing documentation like that. Yeah, but we should all be ashamed of that. (Except you maybe, because you make clean water stuff, but those of us who drive ad clicks should be feeling sort of awkward.)
|
# ? Oct 14, 2014 04:03 |
|
|
# ? Jun 8, 2024 06:54 |
|
Soricidus posted:Could someone who's arguing that tests can make good documentation please post an example of a test suite they'd like to use that way? I'm honestly interested to see what you have in mind. ECMA-262 test is pretty good that way. It's helped by the fact that the alternative documentation is a brain-meltingly hard-to-read specification, though.
|
# ? Oct 14, 2014 04:04 |