On 2023/01/18 18:06, Rowan Tommins <rowan.coll...@gmail.com> wrote: > Then I guess you've never heard people quoting Robert "Uncle Bob" Martin, who > is well known for asserting that code should be self-documenting, and > therefore not need comments, saying things like: > > > The proper use of comments is to compensate for our failure to express > > ourself in code. Note that I used the word failure. I meant it. Comments > > are always failures. > > Many arguments could be had over whether that's going too far, but "too many > comments" can definitely be a bad thing.
There is a tiny piece of truth in that, but it doesn't apply to #include directives. Yes, you should always try to write code in a way that's simple enough for others to understand. But #include directives cannot be written in such a way. Sometimes, you know why an #include is there, but sometimes, you cannot, and only a comment can explain it. > To take the example Dmitry gave: > > > #include "zend_portability.h" // for BEGIN_EXTERN_C > > What should I understand from this comment? That the reason for including this header was because we need BEGIN_EXTERN_C in this file, and BEGIN_EXTERN_C is provided by that header. Without the comment, this would be hard to find out. > - If I want some other symbol defined in zend_portability.h, do I need to > adjust the includes? No, all the symbols are already imported. I could adjust > the comment, but nothing will break if I don't. I don't think that matters much. If you use another symbol from zend_portability.h, you could update the comment, or not. Sure, if you do not, the comment would be incomplete. But that incomplete comment is still better than no comment. > - If I remove the last use of BEGIN_EXTERN_C in this file, can I remove the > include? No, because other symbols from it might be used, even if they > weren't when the comment was written. Removing the last use of BEGIN_EXTERN_C would give you a hint that maybe zend_portability.h does not need to be included anymore. If you care, you can decide to remove the #include and see if it still builds. If it doesn't there's apparently something else still needed from zend_portability.h, and the error message tells you what is needed. If you still care, now's your chance to copy'n'paste that identifier from the compiler's error message to the comment, replacing the old comment. Or just leave the old now-outdated comment. And let somebody else do it, eventually or never. But still, an outdated comment doesn't hurt anybody. It helps keeping the code clean (by being able to easily see #include candidates which can be removed), but if the comment turns out to be outdated, update it or just leave it. I mean, for the last 20+ years, nobody has cared for outdated and unnecessary #includes. Outdated and unnecessary #includes to have negative effects. And now some people pretend to care about outdated and unnecessary comments, which have no negative effects. > - Perhaps the name of the header doesn't give a clear enough idea of what > symbols it might contain, and what types of file should include it. If so, > concentrate on better naming, or better documentation of existing conventions. Yes yes yes! Concentrate on better naming - that's exactly what I'm about to do! And that will lead to FEWER #include comments. For example, many headers include zend_types.h but they only need zend_result. My PR contains a commit which moves this typedef to zend_result.h. Now I can change lots of "#include zend_types.h" to "#inclued zend_result.h", and can then omit the comment, because it's obvious why this header gets included. (Dmitry has complained already about this very commit.) > - Perhaps it is a justification added when the include was first added. If > so, put it in the commit message and PR summary. "Put it in the commit message" can be said about all code comments and all code documentation. I think that's wrong, because: - commit message shows only the initial reason why the #include was added, but unlike code comments, commit messages cannot ever be updated - looking up commit messages is more cumbersome than reading a code comment A commit message should describe why something was done, and how something was done; it is an important part of a software. But it should not replace documentation that helps understand the software. Max -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
