4 ms·
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
by sanderjd 5y ago
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.
- chjj 5y agoI 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.
- defgeneric 5y ago> 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.
- sanderjd 5y agoYour 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.
- b20000 5y agoagree with raw_tx etc.
- chjj 5y agoYou 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.
- nothrowaways 5y agoPythonic
- basedbase 5y agoQuality 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.
- b20000 5y agoif someone needs to review this code and only has a few days to do so, it will be impossible without comments.
- sanderjd 5y agoI 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.
- defgeneric 5y agoNot 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).
- sanderjd 5y ago"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".