Angband Forums

Angband Forums (http://angband.oook.cz/forum/index.php)
-   Development (http://angband.oook.cz/forum/forumdisplay.php?f=10)
-   -   A plea for comments (http://angband.oook.cz/forum/showthread.php?t=4470)

Derakon May 30, 2011 07:14

A plea for comments
 
I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.

I know that the 2.8.3 days had very verbosely commented code; practically every line had a comment. That's not really necessary IMO, but on the flipside, every function more complicated than a getter or setter needs to have a comment. Please, when you modify a function, if you know what it does, comment it! If that function has a complex block within it, comment that too. You'll make everyones' lives easier. Commenting's not really fun, I know, but it'd be a big help in convincing people to contribute.

Timo Pietilš May 30, 2011 07:20

Quote:

Originally Posted by Derakon (Post 53551)
I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.

I know that the 2.8.3 days had very verbosely commented code; practically every line had a comment. That's not really necessary IMO, but on the flipside, every function more complicated than a getter or setter needs to have a comment. Please, when you modify a function, if you know what it does, comment it! If that function has a complex block within it, comment that too. You'll make everyones' lives easier. Commenting's not really fun, I know, but it'd be a big help in convincing people to contribute.

It would also help someone like me who doesn't really have any c-skills and is not interested in coding to make tiny adjustments here and there for personal copies. Old code was easy to read and understand, now I can't make head or tails about whats going on in the code.

Magnate May 30, 2011 10:23

Quote:

Originally Posted by Timo Pietilš (Post 53553)
It would also help someone like me who doesn't really have any c-skills and is not interested in coding to make tiny adjustments here and there for personal copies. Old code was easy to read and understand, now I can't make head or tails about whats going on in the code.

I'm afraid this is one of the unfortunate side effects of having a lot of work done on the code by a large team. Each contributor has different coding styles and a different propensity for comments. We have a set of coding guidelines to keep the layout consistent, but that doesn't help with this particular problem.

Take the menu code for example - it was written by someone with way more expertise than me, whose comments were enough for someone with his/her expertise but not enough for me to understand it. So I can't make head or tail of it - but at least we have a decent menu system. The parser code is a very similar example (though its author happens to hang out on IRC so one can ask questions). I think the better you are at programming in C, the less inclined you are to think something needs a comment.

I do try and add comments where I can, but the catch-22 is that I don't touch the uncommented areas of code very much ...

takkaria May 30, 2011 15:15

Quote:

Originally Posted by Derakon (Post 53551)
I decided to take a stab at allowing ego items to have negative pvals, which requires understanding how the file parser works. This is...not helped by the fact that there are basically no comments for any of the code.

I saw your comment on Trac; the parse_* functions in init2.c follow the form parse_[letter of _info array the info gets parsed into, e.g. e = ego, a = artifact_[letter at the beginning of the line being parsed, e.g. l or n]. Also, for the calls to parser_*, does looking in parser.h help you at all?

Derakon May 30, 2011 18:20

Thanks, Takkaria. Knowing the pattern makes those function names much more accessible, but the pattern is not self-evident -- at least, not to me. I added a diff to that ticket that adds a simple comment to each explaining what it does; the comments may be massively redundant, but that's better than inscrutability.


All times are GMT +1. The time now is 01:09.

Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2020, vBulletin Solutions Inc.