Doxygen comments in C++ source

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Doxygen comments in C++ source

brianlheim
Hey all,

In my last few PRs that modified C++ source, I added Doxygen comments for the functions I added/modified. Since the codebase is somewhat lacking in documentation, I was wondering if we could establish some organizational rules about this. Specifically:

- Any new C++ code must have Doxygen comments
- Any changes to existing C++ code must add Doxygen comments if they're not present
- This applies to all documentable elements - functions, data (such as class members and globals), enums, classes, and files when appropriate
- In header (hpp) files, document the element according to its usage (preconditions, parameters, return value)
- In implementation (cpp) files, document implementation details if necessary
- If the element only exists in an implementation file, document both
- If the element is trivial and can be determined from name alone, it doesn't necessarily need to be documented.

Ideally, over time, this will lead to the most commonly touched parts of the codebase becoming well-documented, making them easier to understand and modify in the future.

Use of these comments was already present (scarcely) in some places in the codebase before I started adding it, so this is not entirely out of the blue. I've worked out an addition to our Travis script that automatically creates the docs and posts them to github-pages:

https://github.com/brianlheim/supercollider/pull/3

You can see the result of that here:

http://www.brianlheim.com/supercollider/index.html

The Doxygen/Javadoc format is widely used and can be parsed by other documentation generation tools such as standardese (https://github.com/foonathan/standardese). Doxygen XML output itself can also be used by other generation tools.

-Brian
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

asynth
I'm long out of active development of SC, but I'll throw in my curmudgeonly opinion here. I hate Doxygen. All it does is give you a laundry list of classes and methods that is mostly useless for figuring out how a framework is architected. And for that you get junked up source code that is harder to read. There is no substitute for taking the care to write a separate document that explains a class, method or framework. Once you have that, you don't need Doxygen. When you are coding and just sticking in inline comments, you are not in the same frame of mind and do not have the context that you have when you are writing documentation. 

Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often don't need to document functions. They say what they do.
Then write a help file.


On Wed, Feb 7, 2018 at 11:02 AM, <[hidden email]> wrote:
Hey all,

In my last few PRs that modified C++ source, I added Doxygen comments for the functions I added/modified. Since the codebase is somewhat lacking in documentation, I was wondering if we could establish some organizational rules about this. Specifically:

- Any new C++ code must have Doxygen comments
- Any changes to existing C++ code must add Doxygen comments if they're not present
- This applies to all documentable elements - functions, data (such as class members and globals), enums, classes, and files when appropriate
- In header (hpp) files, document the element according to its usage (preconditions, parameters, return value)
- In implementation (cpp) files, document implementation details if necessary
- If the element only exists in an implementation file, document both
- If the element is trivial and can be determined from name alone, it doesn't necessarily need to be documented.

Ideally, over time, this will lead to the most commonly touched parts of the codebase becoming well-documented, making them easier to understand and modify in the future.

Use of these comments was already present (scarcely) in some places in the codebase before I started adding it, so this is not entirely out of the blue. I've worked out an addition to our Travis script that automatically creates the docs and posts them to github-pages:

https://github.com/brianlheim/supercollider/pull/3

You can see the result of that here:

http://www.brianlheim.com/supercollider/index.html

The Doxygen/Javadoc format is widely used and can be parsed by other documentation generation tools such as standardese (https://github.com/foonathan/standardese). Doxygen XML output itself can also be used by other generation tools.

-Brian



--
--- james mccartney
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

brianlheim
> I hate Doxygen.

I'm not here to argue about this. What I want is a process that makes it easy to:
- incrementally and reliably improve documentation
- work with a legacy codebase
- organized in multiple directories
- written in many styles
- is in a reasonably flexible format, i.e. can be hooked into other tools and easily cross-referenced with source code

Doxygen is the only tool I know that satisfies all these things and has existing presence in the codebase, which is why I considered it the "default" here. But if you don't like Doxygen, provide an actual proposal for what you do want, because I'll happily work with any tools and processes that satisfy all of the above.

> Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often don't need to document functions. They say what they do.

When the function is a 500+ LOC behemoth with no unit tests and several single-letter-name variables, that's simply not realistic advice.

-Brian

On Wed, Feb 7, 2018 at 3:02 PM, <[hidden email]> wrote:
I'm long out of active development of SC, but I'll throw in my curmudgeonly opinion here. I hate Doxygen. All it does is give you a laundry list of classes and methods that is mostly useless for figuring out how a framework is architected. And for that you get junked up source code that is harder to read. There is no substitute for taking the care to write a separate document that explains a class, method or framework. Once you have that, you don't need Doxygen. When you are coding and just sticking in inline comments, you are not in the same frame of mind and do not have the context that you have when you are writing documentation. 

Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often don't need to document functions. They say what they do.
Then write a help file.


On Wed, Feb 7, 2018 at 11:02 AM, <[hidden email]> wrote:
Hey all,

In my last few PRs that modified C++ source, I added Doxygen comments for the functions I added/modified. Since the codebase is somewhat lacking in documentation, I was wondering if we could establish some organizational rules about this. Specifically:

- Any new C++ code must have Doxygen comments
- Any changes to existing C++ code must add Doxygen comments if they're not present
- This applies to all documentable elements - functions, data (such as class members and globals), enums, classes, and files when appropriate
- In header (hpp) files, document the element according to its usage (preconditions, parameters, return value)
- In implementation (cpp) files, document implementation details if necessary
- If the element only exists in an implementation file, document both
- If the element is trivial and can be determined from name alone, it doesn't necessarily need to be documented.

Ideally, over time, this will lead to the most commonly touched parts of the codebase becoming well-documented, making them easier to understand and modify in the future.

Use of these comments was already present (scarcely) in some places in the codebase before I started adding it, so this is not entirely out of the blue. I've worked out an addition to our Travis script that automatically creates the docs and posts them to github-pages:

https://github.com/brianlheim/supercollider/pull/3

You can see the result of that here:

http://www.brianlheim.com/supercollider/index.html

The Doxygen/Javadoc format is widely used and can be parsed by other documentation generation tools such as standardese (https://github.com/foonathan/standardese). Doxygen XML output itself can also be used by other generation tools.

-Brian



--
--- james mccartney

Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

jamshark70-2

On February 7, 2018 12:36:01 [hidden email] wrote:

>> I hate Doxygen.
>
> I'm not here to argue about this.

(You know who asynth is, right...?)

My 2 cents: Doxygen sucks for reasons I've articulated with respect to SC help files (same problem: writing blurbs about functions and classes is a useful reference but it isn't documentation; in SC-land, we write references but seldom explanatory documents). But, Doxygen would be better than the near-total lack of any explanations of anything. It does carry the risk that we'll dox things without documenting them, though.

hjh

Sent with AquaMail for Android
http://www.aqua-mail.com

Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

asynth
In reply to this post by brianlheim


On Wed, Feb 7, 2018 at 12:34 PM, <[hidden email]> wrote:

> Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often don't need to document functions. They say what they do.

When the function is a 500+ LOC behemoth with no unit tests and several single-letter-name variables, that's simply not realistic advice.

Sure it is. It is the way you make a 500 line function become a number of 25 line functions.

--
--- james mccartney
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

asynth
In reply to this post by asynth
Anyway, I don't want to impose my opinion, just state it. You're free to do things however you like.

On Wed, Feb 7, 2018 at 12:02 PM, James McCartney <[hidden email]> wrote:
I'm long out of active development of SC, but I'll throw in my curmudgeonly opinion here. I hate Doxygen. All it does is give you a laundry list of classes and methods that is mostly useless for figuring out how a framework is architected. And for that you get junked up source code that is harder to read. There is no substitute for taking the care to write a separate document that explains a class, method or framework. Once you have that, you don't need Doxygen. When you are coding and just sticking in inline comments, you are not in the same frame of mind and do not have the context that you have when you are writing documentation. 

Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often don't need to document functions. They say what they do.
Then write a help file.


On Wed, Feb 7, 2018 at 11:02 AM, <[hidden email]> wrote:
Hey all,

In my last few PRs that modified C++ source, I added Doxygen comments for the functions I added/modified. Since the codebase is somewhat lacking in documentation, I was wondering if we could establish some organizational rules about this. Specifically:

- Any new C++ code must have Doxygen comments
- Any changes to existing C++ code must add Doxygen comments if they're not present
- This applies to all documentable elements - functions, data (such as class members and globals), enums, classes, and files when appropriate
- In header (hpp) files, document the element according to its usage (preconditions, parameters, return value)
- In implementation (cpp) files, document implementation details if necessary
- If the element only exists in an implementation file, document both
- If the element is trivial and can be determined from name alone, it doesn't necessarily need to be documented.

Ideally, over time, this will lead to the most commonly touched parts of the codebase becoming well-documented, making them easier to understand and modify in the future.

Use of these comments was already present (scarcely) in some places in the codebase before I started adding it, so this is not entirely out of the blue. I've worked out an addition to our Travis script that automatically creates the docs and posts them to github-pages:

https://github.com/brianlheim/supercollider/pull/3

You can see the result of that here:

http://www.brianlheim.com/supercollider/index.html

The Doxygen/Javadoc format is widely used and can be parsed by other documentation generation tools such as standardese (https://github.com/foonathan/standardese). Doxygen XML output itself can also be used by other generation tools.

-Brian



--
--- james mccartney



--
--- james mccartney
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

julian.rohrhuber
In reply to this post by asynth

> On 07.02.2018, at 22:21, [hidden email] wrote:
>
>
>
> On Wed, Feb 7, 2018 at 12:34 PM, <[hidden email]> wrote:
>
> > Also, by following the principle of: "if you find yourself documenting a piece of code within a function, stop, take that code out into a new function whose name is the text of the comment", you will find you often  don't need to document functions. They say what they do.
>
> When the function is a 500+ LOC behemoth with no unit tests and several single-letter-name variables, that's simply not realistic advice.
>
> Sure it is. It is the way you make a 500 line function become a number of 25 line functions.
Yes, it’s very good advice. It is better to spend more time with trying to find the perfect method name, than to think about how to document it. And better to spend more time with refactoring until it is balanced before documenting it.

Words are often not a good way to explain code anyhow.








signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

Nathan Ho
In reply to this post by brianlheim
On 2018-02-07 11:02, [hidden email] wrote:

> Hey all,
>
> In my last few PRs that modified C++ source, I added Doxygen comments
> for the functions I added/modified. Since the codebase is somewhat
> lacking in documentation, I was wondering if we could establish some
> organizational rules about this. Specifically:
>
> - Any new C++ code must have Doxygen comments
> - Any changes to existing C++ code must add Doxygen comments if
> they're not present
>
> - This applies to all documentable elements - functions, data (such as
> class members and globals), enums, classes, and files when appropriate
>
> - In header (hpp) files, document the element according to its usage
> (preconditions, parameters, return value)
>
> - In implementation (cpp) files, document implementation details if
> necessary
>
> - If the element only exists in an implementation file, document both
>
> - If the element is trivial and can be determined from name alone, it
> doesn't necessarily need to be documented.
>
> Ideally, over time, this will lead to the most commonly touched parts
> of the codebase becoming well-documented, making them easier to
> understand and modify in the future.

hi brian,

my view is this: good code with prose documentation > good code with
generated documentation > good code with no documentation > bad code
with generated documentation > bad code with no documentation. in an
ideal world, all our C++ is pristine and documented with comprehensive
articles. in practice we can settle for lesser alternatives in the
interest of saving work.

so i guess my proposal would be, write Doxygen comments if you really
feel like it, but with the understanding that it's temporary. either
way, code quality comes first.

you, josh, and i have agreed on one thing though: documentation of
sclang/scsynth source should be done in an established documentation
system, not SCDoc. i would like to see all scsynth development tooling
to be self-contained with no dependencies on sclang. (there are vague
plans to phase out SCDoc anyway, but that's a discussion for another
thread) we could use Doxygen for this, and i'm also partial to Sphinx if
we don't mind a Python dependency.


nathan

_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

brianlheim
> so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.

"code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.

To go back to the original point: I don't think that anything in this codebase is going to improve unless we do it incrementally. The best way to accomplish that is by tying those incremental improvements to new changes and additions. That way, we're guaranteed that new contributions can only decrease the technical debt of the project, at least for some obvious but concrete metrics of debt. Is that a controversial statement? From the conversation so far in this thread, my understanding is that people would like to have those incremental improvements be actual refactors and cleanups rather than/in addition to documentation. I'm much happier with that, but it requires significantly more work and discussion, which is why I didn't suggest it in the opening.

-Brian

On Thu, Feb 8, 2018 at 12:36 PM, <[hidden email]> wrote:
On 2018-02-07 11:02, [hidden email] wrote:
Hey all,

In my last few PRs that modified C++ source, I added Doxygen comments
for the functions I added/modified. Since the codebase is somewhat
lacking in documentation, I was wondering if we could establish some
organizational rules about this. Specifically:

- Any new C++ code must have Doxygen comments
- Any changes to existing C++ code must add Doxygen comments if
they're not present

- This applies to all documentable elements - functions, data (such as
class members and globals), enums, classes, and files when appropriate

- In header (hpp) files, document the element according to its usage
(preconditions, parameters, return value)

- In implementation (cpp) files, document implementation details if
necessary

- If the element only exists in an implementation file, document both

- If the element is trivial and can be determined from name alone, it
doesn't necessarily need to be documented.

Ideally, over time, this will lead to the most commonly touched parts
of the codebase becoming well-documented, making them easier to
understand and modify in the future.

hi brian,

my view is this: good code with prose documentation > good code with generated documentation > good code with no documentation > bad code with generated documentation > bad code with no documentation. in an ideal world, all our C++ is pristine and documented with comprehensive articles. in practice we can settle for lesser alternatives in the interest of saving work.

so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.

you, josh, and i have agreed on one thing though: documentation of sclang/scsynth source should be done in an established documentation system, not SCDoc. i would like to see all scsynth development tooling to be self-contained with no dependencies on sclang. (there are vague plans to phase out SCDoc anyway, but that's a discussion for another thread) we could use Doxygen for this, and i'm also partial to Sphinx if we don't mind a Python dependency.


nathan

_______________________________________________
sc-dev mailing list

info (subscription, etc.): http://www.birmingham.ac.uk/facilities/ea-studios/research/supercollider/mailinglist.aspx
archive: http://www.listarc.bham.ac.uk/marchives/sc-dev/
search: http://www.listarc.bham.ac.uk/lists/sc-dev/search/

Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

julian.rohrhuber

> On 08.02.2018, at 19:12, [hidden email] wrote:
>
> > so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.
>
> "code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.


I can help with this on the sclang side, if we decide to work on this. I think it can't be very detailed, because many of the really important factors either are case specific or aesthetic.




signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

amindfv
Without weighing in on what format the documentation should be in, or how mandatory adding it should be, may I just say my heart leaps every time I see a new explanatory comment in the server code. bce7432 and 891fcbc are a couple of nice recent examples.

Tom



On Thu, Feb 8, 2018 at 4:16 PM, <[hidden email]> wrote:

> On 08.02.2018, at 19:12, [hidden email] wrote:
>
> > so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.
>
> "code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.


I can help with this on the sclang side, if we decide to work on this. I think it can't be very detailed, because many of the really important factors either are case specific or aesthetic.




Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

scztt

Following asynth, I hate Doxygen as well - I don't think there's a huge benefit to having a doxygen doc for the SC internals - I rarely find myself searching doxygen dumps for info, unless I don't have the source locally.

*However* --- Brian is right that a "clean camp" policy, asking that people add or clean up doc comments when they visit an area of code, would go along way towards a more navigable codebase. And, if people are putting in the effort of documenting functions / new code, it should probably be standardised, readable, and parseable. The standard for readable, parseable doc comments is probably doxygen. Ergo....

One thing to keep in mind too - Doxygen isn't useful to me, nor is it probably useful to anyone participating in this thread. However, this is an open source project with a wide range of participants and potential-participants. There are *a lot* of contributions and engineering-hours that are impeded or completely blocked because of "what is this argument supposed to be?" kinds of issues - if we can put in a minimal amount of extra effort on this front, it's incremental progress towards a more contributor-friendly codebase.

- S


On Fri, Feb 9, 2018, 9:11 PM <[hidden email]> wrote:
Without weighing in on what format the documentation should be in, or how mandatory adding it should be, may I just say my heart leaps every time I see a new explanatory comment in the server code. bce7432 and 891fcbc are a couple of nice recent examples.

Tom



On Thu, Feb 8, 2018 at 4:16 PM, <[hidden email]> wrote:

> On 08.02.2018, at 19:12, [hidden email] wrote:
>
> > so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.
>
> "code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.


I can help with this on the sclang side, if we decide to work on this. I think it can't be very detailed, because many of the really important factors either are case specific or aesthetic.




Reply | Threaded
Open this post in threaded view
|

Re: Doxygen comments in C++ source

brianlheim
> And, if people are putting in the effort of documenting functions / new code, it should probably be standardised, readable, and parseable. The standard for readable, parseable doc comments is probably doxygen. Ergo....

Yes, this is what I was trying to explain (and not doing a great job of it). There's a lot you get with Doxygen for nearly free, enough so that IMO it doesn't impede other efforts to add documentation. Markdown files can be brought into the resulting documentation very easily, links to related classes and functions are autogenerated. So, it doesn't have to be in-source necessarily.

> There are *a lot* of contributions and engineering-hours that are impeded or completely blocked because of "what is this argument supposed to be?" kinds of issues - if we can put in a minimal amount of extra effort on this front, it's incremental progress towards a more contributor-friendly codebase.

I completely agree. Just today someone was asking me where a certain event was happening in the codebase and I had absolutely no idea where to point them other than to grep the source code. Which isn't very helpful when the function that does this could be named about 5 different things, or the code could be buried deep in a larger function.

So, going back to the original purpose of this thread, here are some more ideas to consider:
- adding summary markdown files to help describe/orient the organization of a directory's contents (where would they live?)
- adding top-of-file descriptions of contents/purpose (I have to say personally this is something that would really work for me)
- for functions that ought to be refactored but aren't for whatever reason, providing a minimum amount of documentation with a TODO for cleanup
- providing better in-source comments for code that might be confusing if you didn't spend the last 5 hours following the same path through the codebase the previous author did

I would also recommend as a long-term goal, getting some sort of unit test system in place for sclang. Of course, better to make the code testable before pinning it down with tests, but my feeling is we'll have to walk a rope between testing for sanity/basic integration while allowing lower-level organization to be easily rewritten if need be.

> > "code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.

> I can help with this on the sclang side, if we decide to work on this. I think it can't be very detailed, because many of the really important factors either are case specific or aesthetic.

Sure! We made a start at this and IMO it was very helpful. I would love to see it continue. There's still this open issue from that wave of consensus-gathering: https://github.com/supercollider/supercollider/issues/2931. It could be a good place to pick the thread back up.

I would take more of a bookkeeping role in those discussions now, I don't have strong opinions on the sclang codebase at this point. But, I do feel that it just makes it so much easier to review and integrate code when the standards are explicit and the conversations about them are referenceable.

-Brian

On Tue, Feb 13, 2018 at 2:02 PM, <[hidden email]> wrote:

Following asynth, I hate Doxygen as well - I don't think there's a huge benefit to having a doxygen doc for the SC internals - I rarely find myself searching doxygen dumps for info, unless I don't have the source locally.

*However* --- Brian is right that a "clean camp" policy, asking that people add or clean up doc comments when they visit an area of code, would go along way towards a more navigable codebase. And, if people are putting in the effort of documenting functions / new code, it should probably be standardised, readable, and parseable. The standard for readable, parseable doc comments is probably doxygen. Ergo....

One thing to keep in mind too - Doxygen isn't useful to me, nor is it probably useful to anyone participating in this thread. However, this is an open source project with a wide range of participants and potential-participants. There are *a lot* of contributions and engineering-hours that are impeded or completely blocked because of "what is this argument supposed to be?" kinds of issues - if we can put in a minimal amount of extra effort on this front, it's incremental progress towards a more contributor-friendly codebase.

- S


On Fri, Feb 9, 2018, 9:11 PM <[hidden email]> wrote:
Without weighing in on what format the documentation should be in, or how mandatory adding it should be, may I just say my heart leaps every time I see a new explanatory comment in the server code. bce7432 and 891fcbc are a couple of nice recent examples.

Tom



On Thu, Feb 8, 2018 at 4:16 PM, <[hidden email]> wrote:

> On 08.02.2018, at 19:12, [hidden email] wrote:
>
> > so i guess my proposal would be, write Doxygen comments if you really feel like it, but with the understanding that it's temporary. either way, code quality comes first.
>
> "code quality comes first" is only meaningful once we have standards on "code quality". Right now the guidelines are very slim and not enough to really amount to quality code, IMO. If you really believe this, then we ought to spend time working on those.


I can help with this on the sclang side, if we decide to work on this. I think it can't be very detailed, because many of the really important factors either are case specific or aesthetic.