3 ms·

That looks clear and concise to me. It's a part of a performance sensitive mission critical infrastructure. The parts you posted are some of the core data stru

by exDM69 10y ago

That looks clear and concise to me. It's a part of a performance sensitive mission critical infrastructure. The parts you posted are some of the core data structures that make it fast by squeezing every bit and byte that reasonably can. There are no points awarded for being pretty. A quick browse around Redis source code looks very clean and elegant to me. There are "hard" low level parts like the stuff you quoted (out of context) and then there are "easy" parts that implement the higher level functionality. It has a decent amount of comments. Taking one of the "nasty" bits out of context is disingenuous. Every computer program has nasty bits and they're usually there for a reason.

deathanatos 10y ago
> That looks clear and concise to me? What's a FRST? The actual code appears to do a LE encoding of a u16, but I can't fathom why you'd call that "FRST". But maybe in context it makes more sense. The rest, IDK. Can't tell what an "LZF hslot bias" is. Again, might make sense in context.
- exDM69 10y ago
  None of this code was meant to be understood without background knowledge of the algorithm internals - and some code just is that way. You don't go reading rbtree.c without first reading about Red-Black Trees in your algorithms book. If I recall correctly, the file you grabbed these from had a lot of comments that thoroughly explain how it works. At least many other files in the redis repo do. They could have named it "FIRST" or whatever FRST is short for. That would have made zero impact to the code quality but a longer name would make things clumsy. It's a file-local helper macro and not a part of any public interface or a crucial piece of the architecture. Feel free to disagree, but picking two nasty lines out of context in an otherwise well designed and implemented code base has zero bits of information about the quality of the project.
  - deathanatos 10y ago
    A URL to the actual source helps, I think, to put it in context: https://github.com/antirez/redis/blob/d680eb6dbdf2d2030cb96edfb089be1e2a775ac1/src/lzf_c.c https://github.com/antirez/redis/blob/d680eb6dbdf2d2030cb96e... The parent poster didn't link, so I lacked the context to really evaluate it. Your persistence at waving this away as "making sense with background knowledge of the algorithm internals" drove me to find it. > If I recall correctly, the file you grabbed these from had a lot of comments that thoroughly explain how it works It's fairly light on comments. Not completely devoid of them, but the code can't stand in isolation. The algorithm itself, LZF, seems to be a variation of LZO, but not notable enough to make Wikipedia. > They could have named it "FIRST" or whatever FRST is short for. The fact that you're unsure of this fact defeats your next point, at least for me: > That would have made zero impact to the code quality Why? One extra letter immediately disambiguates it, and now we know that it at least means "first". > a longer name would make things clumsy If you look at the file it's used in, it has exactly two uses. The readability gained from the extra vowel in this case is well worth it, and would in no way make the code "clumsy". A four-byte (five if you count the commented out code) change so that the reader doesn't need to play guess-the-word? Worth it, in my opinion. The documentation above it too, > just believe me, it works ;) But why does it work? We'll never know, b/c that's bound up in the author's head.

by exDM69 10y ago

deathanatos 10y ago

> That looks clear and concise to me? What's a FRST? The actual code appears to do a LE encoding of a u16, but I can't fathom why you'd call that "FRST". But maybe in context it makes more sense. The rest, IDK. Can't tell what an "LZF hslot bias" is. Again, might make sense in context.

exDM69 10y ago
None of this code was meant to be understood without background knowledge of the algorithm internals - and some code just is that way. You don't go reading rbtree.c without first reading about Red-Black Trees in your algorithms book. If I recall correctly, the file you grabbed these from had a lot of comments that thoroughly explain how it works. At least many other files in the redis repo do. They could have named it "FIRST" or whatever FRST is short for. That would have made zero impact to the code quality but a longer name would make things clumsy. It's a file-local helper macro and not a part of any public interface or a crucial piece of the architecture. Feel free to disagree, but picking two nasty lines out of context in an otherwise well designed and implemented code base has zero bits of information about the quality of the project.
- deathanatos 10y ago
  A URL to the actual source helps, I think, to put it in context: https://github.com/antirez/redis/blob/d680eb6dbdf2d2030cb96edfb089be1e2a775ac1/src/lzf_c.c https://github.com/antirez/redis/blob/d680eb6dbdf2d2030cb96e... The parent poster didn't link, so I lacked the context to really evaluate it. Your persistence at waving this away as "making sense with background knowledge of the algorithm internals" drove me to find it. > If I recall correctly, the file you grabbed these from had a lot of comments that thoroughly explain how it works It's fairly light on comments. Not completely devoid of them, but the code can't stand in isolation. The algorithm itself, LZF, seems to be a variation of LZO, but not notable enough to make Wikipedia. > They could have named it "FIRST" or whatever FRST is short for. The fact that you're unsure of this fact defeats your next point, at least for me: > That would have made zero impact to the code quality Why? One extra letter immediately disambiguates it, and now we know that it at least means "first". > a longer name would make things clumsy If you look at the file it's used in, it has exactly two uses. The readability gained from the extra vowel in this case is well worth it, and would in no way make the code "clumsy". A four-byte (five if you count the commented out code) change so that the reader doesn't need to play guess-the-word? Worth it, in my opinion. The documentation above it too, > just believe me, it works ;) But why does it work? We'll never know, b/c that's bound up in the author's head.