Hacker News Reader

Comment by chjj | Hacker News Reader

Some of the terse variable names are the result of my adherence to a GMP-like naming convention, which I find easy to read and aesthetically pleasing.

The GMP naming convention is something like:

- Pointer/Data - single letter followed by a "p"

- Size/Length - single letter followed by an "n"

So a function declaration might look like:

    static void
    process_bytes(uint8_t *zp, const uint8_t *xp, size_t xn);

The above function would do some processing on `xn` bytes at `xp` and store the result in `zp` (assuming there are `xn` bytes also allocated here).

A function which accepts more inputs might have `yp` and `yn` also, so conceptually: `zp = func(xp, xn, yp, yn);` or to simplify: `z = func(x, y)`.

View on HN

This is a bad convention. Instead of `x` and `z`, you should describe what those pointers are meant to represent. I get that everything is subjective, but some things are actually just bad due to illegibility, and I think it is worth being frank about this.

Quality C code is descriptive in the function name and simply organized, the functions are usually doing one or two things and fairly obvious without many values being passed; you're going to glean a lot more from the function name than the variables. The measure for good C code is extremely different than higher level languages you may be more used to writing.

I believe that this is a cop out, an excuse for a culture of poor conventions. In all languages, good functions should be named well, simply organized, and doing only one or two things, and without many arguments passed in. Also in all languages, parameters and variables should be named expressively. There's no reason C should be exempt from this.

Not really. It's because the interfaces in C tend to follow a common pattern that verbose variable names can get in the way. If we have

  int alter_struct(some_struct_t *s, void *xp, size_t xn);

then it's pretty obvious that the data pointer xp with size xn is going to modify whatever struct object we have at pointer s.

The variable names can be shortened because the function interface follows a common convention.

The really important thing here is a good function name, not a good name for the data pointer argument (a descriptive name could even incorrectly suggest it has a specific type rather than simply point to bytes).

"The data pointer is going to modify the struct" is not the only information that is useful.

Again, it is true in every language that there are some basic patterns to what interfaces look like. That doesn't tell you anything about what the application-specific logic implementing those interfaces is intended to do. For that, it is useful to name things such that they describe what they represent in the domain of the application or library.

If it is a generic function, there are good names for that as well (for instance, some kind of copy function would still have meaningful names like "from" and "to").

But the example given by the author here was not that, it specifically acted on transactions. In that case, the input bytes were meant to represent a "raw transaction" and the output struct is meant to represent a "transaction".

if someone needs to review this code and only has a few days to do so, it will be impossible without comments.

I disagree. The function name gives context as to what they're meant to represent if you understand the convention. One of the conventions in mako is something like:

    int btc_tx_import(btc_tx_t *z, const uint8_t *xp, size_t xn);

This function deserializes a raw transaction of `xn` bytes at `xp` and stores the result in the transaction `z`. Zero is returned on failure.

What would be the alternative here? I suppose I could rename `xp` to `data`, `transaction_data`, `raw_tx_data`, or something like that? I don't think it adds any value and it just takes up extra space, making the code less readable.

Your english language description of it gives some good clues to the alternative:

  int btc_tx_import(btc_tx_t *transaction, const uint8_t *raw_transaction, size_t raw_transaction_size);

Or since clearly `tx` is already a convention for "transaction", it could be `tx`, `raw_tx`, and `raw_tx_size`. And sure, I have no problem with the `p` and `n` stuff, so it could be `txp`, `raw_txp`, `raw_txn`.

But from your description, the input is a "raw transaction" and the output is a "transaction". Using `x` to mean "raw transaction" and `z` to mean "transaction" is obtuse. You know that the input is a "raw transaction" and the output is a "transaction", but I as a fresh reader, don't, and your code does not help me understand.

You make a good point. I will consider changing the names for the import/export functions (but maybe not the MPI code). That or explain the inputs/outputs in detail in docs.

Pythonic

Self documenting code is good in every language.

Self documenting code is not good in BrainFuck.

It would be, if it were possible. That it isn't is why that is a bad language. There are others in that same boat.

agree with raw_tx etc.

> I don't think it adds any value and it just takes up extra space, making the code less readable.

I agree here. What they want is to be able to get a superficial understanding at a glance of what the code is doing--in other words they want to give the absolute minimum effort in terms of reading. But when you actually read/write the code and understand it, the shorter names are an advantage. IMO it comes down to who the names are really important for--the reader/reviewer who will likely move on to something else in the next hour, or the person who has actually given some attention to the meaning of the code?

I think the short names also have the advantage of making the logic of the function body understandable at a glance.

Which code is clearer?

   void f(int c) {
       l.c = c;
       ...
   }

   void setColor(int color) {
       label.color = color;
       ...
   }

Names are more important than matching data types and sizes because they convey intent and meaning better than types.

I just did this grep for single-character function names on the Mako codebase:

    grep -r '\<[a-z](' .

And there were no matches. There were some one-character macros but they were macros repeated dozens of times in a localized area.

It seems like what's being proposed is descriptive function names with simple variable names. If the function bodies are short enough (e.g. fit on a single screen) then this seems like a good trade-off to me. IOW, the variable names are symbolic but the function names are descriptive.

The short variable names should be clear enough if you understand the purpose of the function.

> The short variable names should be clear enough if you understand the purpose of the function.

I shouldn't have to read the function implementation to understand its purpose. Code is buggy! If the function has no name or comment explaining what it's supposed to do, I only have the (often buggy) implementation to go by.

There is no reason to use terse, non-descriptive names in 2021. It's an awful practice that guarantees easy-to-avoid bugs.

I am saying you indeed should have descriptive function names.

I also agree that if the function's name leaves something to be desired then it should be commented.

You are conflating function names--global and relatively non-contextual--with variable names--which have limited scope and rely on the function name for their meaning.

In the setColor example, I would use setColor for the function name and c for the parameter name (with the caveat that C language doesn't have method names, so my reasoning about context has limited applicability to non-C languages)

"Clean code reads like prose" (Uncle Bob C. Martin)

This is quickly becoming my goto standard for measuring how clean my code is, and in my case this means ultra-descriptive variable names. I usually code in two passes: first rough things out using single-character/short names, and then go back and use LSP features to rename the variables using the language-aware tools in any modern code editor.

This is silly. The variable types are already giving you exactly this information. Why would you add a "p" when you already have the "*"? Likewise, size_t tells you that this is a size, no need for the "n".

Instead, the variable names should be used to convey information that the types alone can't convey.

> Likewise, size_t tells you that this is a size, no need for the "n".

How do you differentiate the two input lengths? If I were to rename `xp` to `x` and `xn` to `n`, what should `yn` be renamed to? At the very least, there's going to need to be a `yn` somewhere.

It's very common for code to include the type when there are two inputs to a function (even when written more verbosely): e.g. `thing_len`, and `other_thing_len`.

The `p`-suffix convention can also save you in a situation like this:

    int x = 1;
    int *xp = &x;
    int y = 1;
    int *yp = &y;

It avoids naming collisions, and further down in the function, you'll be able to differentiate the pointer and the value. I find it very useful.

If you write multi-precision integer code in C[1] without this convention, you will end up with an unreadable mess. I certainly wish Torbjörn Granlund were here to testify to this.

[1] https://github.com/chjj/mako/blob/master/src/mpi.c

I don't understand why you chose to prefix your variables with x and y rather than something more descriptive. It seems like x and y are completely arbitrary, which is confusing.

Reply on news.ycombinator.com