Comment by alerighi | Hacker News Reader

Comment by alerighi | Hacker News Reader

Parent Full thread

If the name of the function and the parameters are clear, documenting the code seems useless.

Take this real example:

    /**
     * change the temperature set point in use by the thermostat
     *
     * @param ctx the thermostat context
     * @param sp the new set point in 0.1 celsius degrees
     * @return 0 on success, < 0 in case of an error
     */
    int change_sp(void *ctx, int sp);

But we can change around the name of the parameters like this and use proper strict types:

    /**
     * change the temperature set point in use by the thermostat
     *
     * @param thermostat_ctx the thermostat context
     * @param set_point the new set point
     * @return SUCCESS on success, otherwise an error code
     */
    error_t update_temperature_set_point(thermostat_ctx_t *thermostat_ctx, celsius_degree_t set_point);

And thus the doc comment now it's useless and can be removed, leaving only:

    thermostat_error_t update_temperature_set_point(thermostat_ctx_t \*thermostat_ctx, celsius_degree_t set_point);

And in a codebase, more than 95% of the comments would be like that. There are the exception where you need to explain something in more details. In that case you first should ask yourself if there is really not a better way, and if not in that case the doc comment is fine. In all the other cases, it's probably not.

Doc comments on the other hand really makes the code less readable, the increase the number of lines for most of the times not saying anything useful at all.

Why are method names priveleged over comments? They're both text read for human understanding. Why must the full documentation/definition for a function fit into the length of a method? These are silly restrictions, and lead to worse code.

Because writing a(a,b) is not ok even if comments are great, so why not make the method name a clear enough verb that comments are often unnecessary.

The problem is that not everyone code under the same circumstance, even inside the same codebase: you have the new joiner that has to reverse engineer most of what he uses, the senior who did or touched enough that he doesnt even see the color code for comments, the rushing helper who need to fix a bug now or else and cant maintain comments or he'll miss the deadline, etc.

Comments are sacrified first, tests seconds, code clarity third, correctness in edge cases next, etc. Embracing the reality that we're going to have to deal with crap once in a while would solve a lot of frustration.

Accepting that comments are sacrificial does not mean sacrificing them eagerly. In general, code with comments is better than code without.

Because what the code does should be obvious without looking at the documentation. Plus editors will autocomplete function names by looking at the name, not at the documentation. Thus using meaningful names can speed up the development.

This is maybe OK until you get a library user who doesn’t know what “ctx” means and now you have another Discord ping or GitHub issue. Just write the documentation.

what is "set point"? Is it a point that is a part of some kind of set? What's "point" - is it part of some kind of coordinates?

You are documenting _what_. To document _why_, you must explain why somebody would want to change the "set point" and what is the effect, and not in terms that "setA sets a", but the semantics of it. What does setting point accomplish? What are the circumstances when you want to do it? _WHY_ would somebody want to call this method? Make the room colder? Make the room hotter? That kind of thing.

You have to assume a certain amount of domain knowledge, or you end up explaining the entire universe.

In this case, "set point" is a well-understood term, and its purpose is similarly well-understood. [1] Although the team may wish to document it, the place to do so is in the team glossary (or other overview documentation), not in every function that uses it.

[1] http://faculty.washington.edu/brengelm/neut_zone/pg3.html

Thanks for info, I did not know it's a common industry term!

Anyway, the point I am driving is, it's more interesting to discuss _why_ would anyone change the "set point" - common scenarios and pitfalls, than just explaining the "update_set_point changes the set point".

It's nice to go into details what are the consequences and common use-cases.

> But we can change around the name of the parameters like this and use proper strict types

Sometimes. Other times proper strict types aren't possible because the language doesn't support them (many scripting languages, C, etc.)

Assuming this is C, even a type like "celsius_degree_t" tells me nothing about the valid range. Am I supposed to just try it out and see what error code I get or are we back to looking at the implementation? In the case of C this will most likely involve locating and opening a completely different file whose name doesn't even have to be related to the header file.

The comment also doesn't need to be about all parameters and can contain additional information that's non-trivial to determine: is the function thread-safe, are there performance implications, is there notable resource usage, and so on.

Python in particular is notorious for having optional (keyword-) arguments and polymorphic arguments. Proper naming schemes are impossible in this case (lest you accept monstrosities like "node_as_id_or_name_or_object" as proper argument names) and type annotations have only very recently (as of 3.10) become somewhat sane. Type annotations don't help with kwargs, though.

TL;DR it greatly depends on the programming language and its capabilities whether naming and types alone can replace comments.

That's more of an argument for them being overused, not for them being useless.

It's an argument for a certain code style, characterised by descriptive names and specific types. This calls for a language capable of e.g. doing maths with user-defined types, which not all languages can do.

I think one of the major motivators for the original style of code are legacy pressures and a concern about code width.

If every other function that manipulates temperature set points takes "int sp", then you're going to get a lot of pressure for consistency. A lot of programmers regard consistency as a goal in and of itself, and it's very easy to demand it during a code review. However, the demand for consistency prevents us from finding a new consistent target without excessive amounts of work. It may be better to reach a new consistency within a narrower scope, as long as a consensus has been reached to extended that consistency outwards. In this system, consistency should be viewed as a compromisable target: something that makes you ask a careful question. And when asking a question, take into consideration the propensity of the code author to view a question as a demand. If you're a senior and the code author is a junior, presume that means: ask a genuine question vocally; write down minutes of your discussion.

The code width concern is a bit silly, but for some reason we assume that things get weird when lines exceed 80 characters. That's easy to do.

You’re missing under what conditions an error is thrown. I used your method now I got an obscure error code. I’ll change it - whoops a different error code. Documenting it would let someone know what values of set_point are valid and what context states work/don’t.

Reply on news.ycombinator.com