News:

Do you need help?
Simutrans Wiki Manual can help you to play and extend Simutrans. In 9 languages.

Improving code comments + doxygen support

Started by eipi, October 28, 2013, 05:22:55 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

eipi

Hi,

I'm currently working on improving and translating code comments, improving doxygen support along the way.
This first patch is for fabrikbauer_t.
Suggestions / Comments appreciated :)

Markohs

good good, documentation is allways welcome, thx :)

Dwachs

Parsley, sage, rosemary, and maggikraut.

eipi

OK, patch updated to r6873.
I fixed some whitespace and removed the @author tags.

Dwachs

Parsley, sage, rosemary, and maggikraut.

eipi

Patch update to r6881:
Removed the rest of the @author tags and fixed my own wrong documentation. ;)

Max-Max

Quote from: Dwachs on October 29, 2013, 01:03:56 PM
I personally prefer to have the documentation in the header.
Try to put your self into this situation:
You are a beginner in this project and just started to debug the code. The majority of all functions are in the .cc file and this is where you end up debugging. To read about each function you enter you have to track down the header file and search for the function's documentation, if there is any...

It is by far more practical to have the documentation in the .cc file, so you can read it immediately while debugging. It also divides the function blocks distinctively.

Quote from: Dwachs on October 29, 2013, 01:03:56 PM
The @retval tag was unknown to me, can be used as well.
Glad to be of assistance ;)

Quote from: Dwachs on October 29, 2013, 01:03:56 PM
The @author tag does not give much information: it names the creator of the sub-routine, after some time a lot more people modified the routine, so the notion of  'author' loses any sense here.
True indeed. Usually the authors are listed in the file description at the top of the .cc file. I think that people who contributes would be mentioned some how as a token gratitude of their work. I don't see any need to tag this in Doxygen although.
- My code doesn't have bugs. It develops random features...

Ters

Quote from: Max-Max on November 02, 2013, 11:59:16 PM
Try to put your self into this situation:
You are a beginner in this project and just started to debug the code. The majority of all functions are in the .cc file and this is where you end up debugging. To read about each function you enter you have to track down the header file and search for the function's documentation, if there is any...

It is by far more practical to have the documentation in the .cc file, so you can read it immediately while debugging. It also divides the function blocks distinctively.

While my initial instinct is to put the documentation along with the definition of the function (which may be either in a .cc file or in a .h file), there are some good counter arguments:

When writing code, or just reading it, you have references to the header files, so it's easy to go to the header files and read about the functions you are using. While there usually is a one-to-one relationship between headers and source files, and they share the name, that is not always the case. So it's easier to get to the header declaring (or defining) the function, than to the source file defining it. This becomes even more important for library projects, as users may only have (direct) access to the header files, but that's not the case for Simutrans.

When debugging with GDB, I only see a few lines of code around the function I'm in anyway. Even if I tell it to show be the entire function, I think it starts at the signature. Inline comments may also be more important to understand how the code works than doxygen blocks, which describe the function as a whole and therefore tend to describe what the code does and how to use it, not how it works.

Another argument against putting comments in header is that it slows down compilation time, as much more text has to be included into every source file using that header. Without comments for each member variable and function, it's also easier to get a quick glance at a class through it's definition in the header, but only as long as there are no inline function definitions longer than a single line. However, member variables can only really be documented in the header file.

prissi

The only think which is tricky with header is with overlaid virtual fuctions. MSVC usually jump to the wrong definitions in those cases.

(It often refuse to jump from definition to declaration or backwards without reasons, but this is an entirely different problem.)

Markohs

Quote from: Max-Max on November 02, 2013, 11:59:16 PM
Try to put your self into this situation:
You are a beginner in this project and just started to debug the code. The majority of all functions are in the .cc file and this is where you end up debugging. To read about each function you enter you have to track down the header file and search for the function's documentation, if there is any...

Sure, and if a begginer is looking at sourc code and trying to understand a class he'll prolly look at the .h, and the definitions won't be there according your criteria.

There is no magic solution, ofc. But if we agreed on documenting the .h and only the .cc if it makes sense for the specifics of the implementation, why changing it *again*, when there is no *obvious* advantage, just for the sake you like it more that way, not any other substantial advantage?

I'm open for change, and refactoring and re-design. Everything that makes code better, but why wasting time on this insubstantional issues and discussions? Is all this really worth it to someone?

prissi

Isn't there a nce refactoring tool to keep both consistent, or show the documentation at definition when on the declaration. MSVC cannot do this as far as I know (or are there addons)?

Max-Max

Quote from: Markohs on November 04, 2013, 12:47:48 AM
Sure, and if a begginer is looking at sourc code and trying to understand a class he'll prolly look at the .h, and the definitions won't be there according your criteria.
Yes, that is why I personally like a clean header file. Things that can't be documented in the .cc file goes into the header file.
Where do you spend most time in the code, the .h or .cc file? I'm just sharing my experience of many years debugging in various projects...

Quote from: Markohs on November 04, 2013, 12:47:48 AM
I'm open for change, and refactoring and re-design. Everything that makes code better, but why wasting time on this insubstantional issues and discussions? Is all this really worth it to someone?
Yes, it is to me as a "beginner" in this project. That was why I brought it up to begin with. This doesn't mean that we have to make a project and convert all files to a new doxygen standard, I think it is simply enough to document a standard and follow it when we do document. If we are somewhere editing we can move some comments if we feel too as well and slowly we will get updated.

I don't think I ever have come across a project where all documentation was in the header file anyway.
- My code doesn't have bugs. It develops random features...

Ters

Quote from: Max-Max on November 04, 2013, 01:08:46 AM
Where do you spend most time in the code, the .h or .cc file?

For Simutrans, I might spend most time i .cc files, but I open and read mostly .h files.

Quote from: Max-Max on November 04, 2013, 01:08:46 AM
I don't think I ever have come across a project where all documentation was in the header file anyway.

I can't say I have either, but I've seen some where all the formal documentation is in the header files, because the source files are not (necessarily) available. But this was library projects.

Markohs

Quote from: Max-Max on November 04, 2013, 01:08:46 AM
Yes, that is why I personally like a clean header file. Things that can't be documented in the .cc file goes into the header file.
Where do you spend most time in the code, the .h or .cc file? I'm just sharing my experience of many years debugging in various projects...

Well, you will agree there is no perfect solution to this. And looking at the .h it's clearly not a drama (right-click, Go to declaration). Do you all want to move documentation to the .cc? Do it. Do you want to move to the .h? Do it. I just don't care. But take one decision and be consequent with it.

And if you really want to move all the doxy to the .cc you should also consider where will you end describing the classes.

What I don't think it makes absolutely any sense is just exploring the code and at every step you don't like something, try to change it. We are just discussing one hundred small issues instead of doing what we should be doing, that's actually do something useful with the code. I can understand the discussion about the color, that's something useful, not this one imho. Why changing something that's already working.

Will you offer yourself to move all the doxygen already present in the .h to the .cc files? I'd be great to have all code follow some rule.

And I'm saying this from the respect, not being rude, I'm just pointing my oppinion.

I'd just rather prefer to do something of relevance, but opening old discussions that were more or less resolved one or two years ago. But seeing how are things working I'd not be surprised if in the future someone wants to move comments to the .h again, and two years later to the .cc again. What will this end in? No comment will be correct and in its place, ever.

As Ters menctioned documenting the .h is maybe more suitable for libraries, but it's useful in the whole code too, and hey, if someone wants to be able to read doxygen while debugging... Well, I'd like also to have a small (5 cm tall) elf living on my desk pointing me the bugs on screen with his finger but well, you can't have it all in life. ;)

kierongreen

We agreed documentation generally goes in .h, respect that rather than continually arguing against consensus opinion. Please.

Dwachs

Quote from: eipi on November 02, 2013, 09:16:04 PM
Patch update to r6881:
Removed the rest of the @author tags and fixed my own wrong documentation. ;)
thank you, incorporated now.
Parsley, sage, rosemary, and maggikraut.

An_dz

On r6907 simgraph16.cc includes two new comments that I can't understand, on lines 2364 and 2374 it reads touced, but what is touced?

My best match is that it should be toused.

Can someone please 'fix' this?

eipi

My guess is it should say touched, i.e. the h is missing ;)

eipi

New patch on the way...  ;)

This patch fixes up comments in hausbauer.h/.cc. I also moved the house lists to a single place in the .h.

Dwachs

Parsley, sage, rosemary, and maggikraut.

eipi

@dwachs: Thanks for incorporating.


While I'm at it, here's a patch for tunnelbauer_t updating all the comments.
Suggestions on how to improve them are welcome, as always. :)