3 ms·

I suppose I'd grant that there's two distinct audiences: one who's familiar with the codebase already and has to read these symbols but knows what they mean, an

I suppose I'd grant that there's two distinct audiences: one who's familiar with the codebase already and has to read these symbols but knows what they mean, and one who's new to the code. I think your point only works for the former and doesn't help the latter at all. So we should weigh the negative impact to the familiar-reader of scanning a 64-character identifier distinguished only by a suffix like "_gr" (or "_greater" if that's what is meant here) against the negative impact of the difficult-to-decipher-abbreviations to the unfamiliar-reader. IMO the net win is to optimize for the unfamiliar-reader in this case. The frequency of unfamiliar-readers is much lower than familiar-readers, the positive impact is much more significant. > As to this particular function name, you're in MP3 land, so MDCT means Modified Discrete Cosine Transform. The "i" is probably "inverse." "L3" is probably "Layer 3" since we're talking about MPEG Layer 3. In context, it's not actually a bad function name. Agreed, this codec implementation will likely use some abbreviations for sane reasons. But even if you know what mdct/l3 mean in this context (I did), can you say what this function does (or should do) by looking at the name? What about it is distinct from L3_imdct12/L3_imdct36/etc?

lucideer 8y ago
> I'd grant that there's two distinct audiences: one who's familiar with the codebase already and has to read these symbols but knows what they mean, and one who's new to the code. I think your point only works for the former and doesn't help the latter at all. With the exception of the "i" (possibly inverse?), the rest of the parts seem like things you would very quickly become familiar with after a short period of looking at the code and reading up on the subject matter (both of which would be required if you wanted to contribute or port). I guess having names that are immediately understandable to any fresh reader with zero domain knowledge is a noble goal, but it's a high bar. It's not a perfect function name (you've pointed out some other, valid, problems) but it's far from being objectively bad.
- tjoff 8y ago
  > I guess having names that are immediately understandable to any fresh reader with zero domain knowledge is a noble goal, but it's a high bar. I'd argue that it is often detrimental to the readability for those with domain knowledge, and if the code (as often is) requires the domain knowledge it can be a disservice. Consistency is key, if you are consistent these shortened function names can be disturbingly pleasant. Long names are sometimes a consequence of the programmer not taking the necessary time to think about it (also, consistency is still paramount).
rayiner 8y ago
There is a difference between being unfamiliar with the codebase (I've never seen this code or the MP3 spec before), and being completely unfamiliar with the problem domain and file format. I don't think it is helpful to write an MP3 decoder assuming that the reader doesn't know about one of the fundamental mathematical operations or the basic unit of the bitstream format. YMMV, but I personally find names like this helpful. It ties the code to the underlying problem domain. E.g. I'm not exactly sure what imdct12/imdct36 are, other than the fact that they operate on a block of data smaller than a granule, but I bet if I opened up the MP3 spec the "12" and the "36" would be more helpful in finding the relevant section than other names that could've been used. EDIT: In fact, the MP3 format defines two MDCT block sizes: 12 points and 36 points.
ori_b 8y ago
> So we should weigh the negative impact to the familiar-reader of scanning a 64-character identifier distinguished only by a suffix like "_gr Having a 64 character identifier is the bigger problem here.
- wyldfire 8y ago
  shrug, namespaces go a long way to mitigating that one. C's great but we've got a lot better options these days.
jacobolus 8y ago
An MP3 “granule” (according to web search) is 576 points long. so L3_imdct_gr, L3_imdct36, and L3_imdct12 are going to be inverse modified discrete cosine transforms operating on 576, 36, and 12 coefficients, respectively. Modified discrete cosine transform means a DCT type 4 in which adjacent blocks are overlapped. See https://en.wikipedia.org/wiki/Modified_discrete_cosine_transform https://en.wikipedia.org/wiki/Modified_discrete_cosine_trans...

lucideer 8y ago

> I'd grant that there's two distinct audiences: one who's familiar with the codebase already and has to read these symbols but knows what they mean, and one who's new to the code. I think your point only works for the former and doesn't help the latter at all. With the exception of the "i" (possibly inverse?), the rest of the parts seem like things you would very quickly become familiar with after a short period of looking at the code and reading up on the subject matter (both of which would be required if you wanted to contribute or port). I guess having names that are immediately understandable to any fresh reader with zero domain knowledge is a noble goal, but it's a high bar. It's not a perfect function name (you've pointed out some other, valid, problems) but it's far from being objectively bad.

tjoff 8y ago
> I guess having names that are immediately understandable to any fresh reader with zero domain knowledge is a noble goal, but it's a high bar. I'd argue that it is often detrimental to the readability for those with domain knowledge, and if the code (as often is) requires the domain knowledge it can be a disservice. Consistency is key, if you are consistent these shortened function names can be disturbingly pleasant. Long names are sometimes a consequence of the programmer not taking the necessary time to think about it (also, consistency is still paramount).

rayiner 8y ago

There is a difference between being unfamiliar with the codebase (I've never seen this code or the MP3 spec before), and being completely unfamiliar with the problem domain and file format. I don't think it is helpful to write an MP3 decoder assuming that the reader doesn't know about one of the fundamental mathematical operations or the basic unit of the bitstream format. YMMV, but I personally find names like this helpful. It ties the code to the underlying problem domain. E.g. I'm not exactly sure what imdct12/imdct36 are, other than the fact that they operate on a block of data smaller than a granule, but I bet if I opened up the MP3 spec the "12" and the "36" would be more helpful in finding the relevant section than other names that could've been used. EDIT: In fact, the MP3 format defines two MDCT block sizes: 12 points and 36 points.

ori_b 8y ago

> So we should weigh the negative impact to the familiar-reader of scanning a 64-character identifier distinguished only by a suffix like "_gr Having a 64 character identifier is the bigger problem here.

wyldfire 8y ago
shrug, namespaces go a long way to mitigating that one. C's great but we've got a lot better options these days.

jacobolus 8y ago

An MP3 “granule” (according to web search) is 576 points long. so L3_imdct_gr, L3_imdct36, and L3_imdct12 are going to be inverse modified discrete cosine transforms operating on 576, 36, and 12 coefficients, respectively. Modified discrete cosine transform means a DCT type 4 in which adjacent blocks are overlapped. See https://en.wikipedia.org/wiki/Modified_discrete_cosine_transform https://en.wikipedia.org/wiki/Modified_discrete_cosine_trans...