-
Notifications
You must be signed in to change notification settings - Fork 776
Add: Doxygen documentation comment for many of the classes. #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
That seems to be very useful. It would be good though, if some of the developers could quickly check the comments, if they are all correct. It is easy to misunderstand things, and wrong documentation would be even worse than no documentation. Additionally, I would vote for a somewhat 'minimalistic' comment style to avoid redundancy. Instead of documenting absolutely everything, I would only comment the interesting things. If a class is called "Logger", it is completely redundant to describe it as "A Logger class", it tells you nothing more than the name already did, but makes the documentation harder to understand (more reading for an equivalent amount of information). But as correcting something is usually far easier than creating something, this should be a valuable start for documented code. |
|
Good idea, but most of the comments added are trivially correct. Two places you had wrong understanding: |
|
Both points fixed. Changes:
@JKleinHub: I agree, but the question is, what is "interesting"? Different people have different ideas about that. The only solution I see is to just do everything. It avoids "what is interesting" discussions, and takes pretty much no time at all, as trivial things are trivial to add. (Note that by definition, an author that writes the code has a deep understanding of it, and thus finds everything trivial, otherwise he cannot write it. This is the whole problem of documentation, At that point in time, he is the only one that can write documentation real fast, yet he has no use for it at that moment, as he understand everything. Unfortunately, such knowledge is not automagically transferred to other people nor to the future himself.) In my case, I just switched to blindly typing doxygen comments everywhere without much thinking, and only now can look again to see if I can find more structure. @xlz: For me, a buffer is a char * address, things like length and capacity is meta data. However, it's not something to make a fuss about. As for the noise, I got that from line 232 "// check for allowed depth noise". Maybe that should be changed too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, this function maps color pixels onto depth data, so you get a 512x424 color image. If the bigdepth buffer is provided, it also performs the inverse mapping. This means the buffer is filled with depth pixels mapped onto color data, so this gives you a 1920x1082 (yes, 1082) depth image with one blank row on the top and the bottom (i.e. (float*)(bigdepth->data)[1920] is the offset of the first color-mapped depth pixel).
|
Changed as follows: |
|
I think commenting would be after the restructure and be limited to public API only. Some of the trivial or uninformative comments would be avoided if they are only for internal code. C++ itself is already a verbose language. The comments do not give more information than just reading the source code. Give you an example:
|
|
Comments are highlighted differently, and contain only the high level information. "Footer of a depth packet." says exactly what I need to know whether I can skip the next 13 lines to the next comment. The C++ code has a lot of clutter "LIBFREENECT2_PACK(struct LIBFREENECT2_API" jumps out here due to all the upper case words, while "Depth" and Footer" are hidden in a very long word. You have to carefully read, parse the line, and then throw away the right 80% of crap, to find the words you need. Even then, maybe "Depth Sub Packet" is significant? The comment tells, the code doesn't. So even "trivial" comments help, you sprinkle fast skipping points in the code, pointing out the significant words, guiding the reader. For the discussion of what is trivial, I have been reading the code for a day, so sure I am in no position to make deep non-trivial additions to the documentation. On the other hand, "bigdepth" that I changed just above was completely undocumented so it was considered trivial too? While you may know what kind of frame to insert into it, it is far from trivial, you have to reverse engineer the code. The +2 lines is not easy to spot. The fact that there is supposed to be 1 blank line above and below it in the result isn't explained either. I would need an hour or so at least and still wonder whether that first blank line is supposed to happen, or whether it is a co-incidence. You can write that line of text in 5-10 seconds. I can read that line in 5 seconds, understand what it says, be sure I have the right solution (for as far as I trust the documentation), and do one hour more useful stuff (eg make a contribution), but I need that line of text for that. You can read code, but it's always slower than reading a few lines of text. (Even for you in a year from now, do you remember all the little details of all code you ever wrote? I don't think so, at least I don't.) In the code, you may miss crucial details, and you're never sure your analysis of the code was deep enough to find all the edge cases that may occur. The correct solution means a working solution, but a working solution does not imply correctness. In the end, documentation is about your readers, not about you. Just like you make a report or paper easy to read, documentation makes code more accessible for your readers. The main difference is that code also happens to be readable by a machine. If you don't want to explain how nice the solution is that you implemented in an accessible way and instead have your readers struggle for days through all the little details of each line of unknown code, I think it's a missed opportunity for you, and a big waste of precious time at global level in the open source world. But having said that, I needed to make the documentation to understand the structure, and find crucial details that I missed the first two passes. I offer my work to the project. It's up to you whether you find it useful enough to add. |
|
Well, I think it's absolutely useful and will merge it soon, after #368 - you may need to re-merge your PR, but it shouldn't require a lot of changes. |
Add: Doxygen documentation comment for many of the classes.
While trying to understand the classes, I wrote some Doxygen comments, which may be useful for you.