7 ms·

Not Ruby but for C code, I like Redis's code: https://github.com/antirez/redis/tree/unstable/src https://github.com/antirez/redis/tree/unstable/src I don't pe

by rdtsc 10y ago

Not Ruby but for C code, I like Redis's code: https://github.com/antirez/redis/tree/unstable/src https://github.com/antirez/redis/tree/unstable/src I don't personally use the product, but I find the source well written and always share it as an example of nicely done C code. Here is some nice Erlang code I like -- network packet parsing: https://github.com/msantos/pkt/tree/master/src https://github.com/msantos/pkt/tree/master/src Notice how concise this is: codec( <<4:4, HL:4, ToS:8, Len:16, Id:16, 0:1, DF:1, MF:1, %% RFC791 states it's a MUST Off:13, TTL:8, P:8, Sum:16, SA1:8, SA2:8, SA3:8, SA4:8, DA1:8, DA2:8, DA3:8, DA4:8, Rest/binary>> ) when HL >= 5 -> ... This is due to the beauty of binary pattern matching. You could kind of do it in C by casting to a struct but notice here it is also calculating header length (HL) as part of the match operation. So it can do a bit more than just casting a binary blob to a struct. Another thing here is that it is also big endian by default so there is not need for htons(), htonl() and such functions sprinkled throughout the code.

ebbv 10y ago
Not disputing the code quality of either above project, but over valuing brevity / being concise is a common cause of problems. Specifically, people will often sacrifice readability and robustness for brevity. At this point in my career I don't really value being concise at all. Which is not to say you shouldn't ever think about "Can I write less code here?" but the goal there should be writing less code to make what you're writing more robust, more stable, more readable and not just shorter. If there's one thing I could tell every programmer early in their career (including a younger me) it would be; nobody's going to be impressed with how clever you were when they're frustrated trying to understand what the heck is going on. Including you in 6 months.
- rdtsc 10y ago
  There is a difference though when it is something custom and new (say if this code defined or parsed a new protocol) vs parsing an existing protocol. Notice https://tools.ietf.org/html/rfc791#section-3.1 https://tools.ietf.org/html/rfc791#section-3.1 section diagram. That code is basically an almost perfect translation of the RFC diagram, down to the bits involved. That is the beauty here: minimal impedance mismatch between the specification and code.
  - Denzel 10y ago
    You can eliminate the impedance mismatch entirely by using the specification as your code [1], as Alan Kay's Viewpoints research team did. ;) [1]: http://www.moserware.com/2008/04/towards-moores-law-software-part-3-of-3.html http://www.moserware.com/2008/04/towards-moores-law-software...
    - rdtsc 10y ago
      Ha! Interesting, just feed the diagram to the code.
- matt_wulfeck 10y ago
  It's amazing that this is still highly debatable in our industry. Always write to make debugging easier. Always. There's so many people that worship cleverness over readability.
  - xelxebar 10y ago
    In all fairness, a stunningly clever insight can make things seem so obvious that you forget how hard it was to have that original insight. This kind of brevity is extremely helpful when thinking about a problem. You "just" have to grok the mechanics thoroughly. This really helps in designing effective higher level abstractions. Unequivocally admonishing brevity for ostensible readability kind of ignores the trade offs. I think there are other ways to bootstrap grokkability without having to sacrifice the conceptual clarity of nicely compact code.
    - qorrect 10y ago
      As someone taking a lot of ML/AI courses ( a subject new to me) , these clever insights are popping up all around me lately, and I agree once you learn it, it's hard to imagine what it was like before. I'm hitting a lot of 'aha' moments which has really been emotionally satisfying, it's been a long time since I had to push myself I guess.
- sudshekhar 10y ago
  As a side, what are some good ways to start understanding the bigger projects? Blogs and documentation are helpful but mostly don't talk about the underlying implementation details. Digging through the code line by line takes a long time and while one does learn a lot, I am sure there has to be a better way of doing this. Any static analysis tools that help in this regard? Or any other tool/approach that you might recommend?
- sealjam 10y ago
  I am leaving my current team, in part for this very reason. Senior technical people in the organisation don't value readable, understandable code, favouring 150 character one-liners instead. It's proven to be a very effective way of increasing the time taken for anybody except the author to debug issues. In my view, this has increased the perceived value of those responsible. I'm sure that wasn't their ultimate intention but there's certainly no incentive to change!
danbruc 10y ago
The Redis code doesn't look exceptionally good to me. I just looked through five files, but things like spacing and the use of braces seem to be pretty inconsistent. There are also a lot of magic numbers and quite a few variables have poor names.
- rdtsc 10y ago
  Noticed those too, also I don't personally like camelCase in C however I still like the code because of how it is laid out and solves the problem. It has the right amount of comments (too many of those is also bad), some functions are large but overall has the a good balance there too. Also for an example it is also the right sized project. It i not too small and not huge (like Linux kernel) and the overall structure can be understood in a few hours of browsing through.
- bryanrasmussen 10y ago
  I agree that things like spacing and inconsistent use of braces may cause code not to look good, but don't really feel it is a surefire indicator of the code not actually being good.
singularity2001 10y ago
#include "lzfP.h" # define FRST(p) (((p[0]) << 8) | p[1]) ref = hslot + LZF_HSLOT_BIAS; hslot = ip - LZF_HSLOT_BIAS; I pity all C programmers who have to deem this beautiful
- 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.
mapcars 10y ago
What you show in Erlang example is just a syntax. The surrounding code or usage or approach may be bad/ugly. It has nothing to do with code quality.

by rdtsc 10y ago

ebbv 10y ago

Not disputing the code quality of either above project, but over valuing brevity / being concise is a common cause of problems. Specifically, people will often sacrifice readability and robustness for brevity. At this point in my career I don't really value being concise at all. Which is not to say you shouldn't ever think about "Can I write less code here?" but the goal there should be writing less code to make what you're writing more robust, more stable, more readable and not just shorter. If there's one thing I could tell every programmer early in their career (including a younger me) it would be; nobody's going to be impressed with how clever you were when they're frustrated trying to understand what the heck is going on. Including you in 6 months.

rdtsc 10y ago
There is a difference though when it is something custom and new (say if this code defined or parsed a new protocol) vs parsing an existing protocol. Notice https://tools.ietf.org/html/rfc791#section-3.1 https://tools.ietf.org/html/rfc791#section-3.1 section diagram. That code is basically an almost perfect translation of the RFC diagram, down to the bits involved. That is the beauty here: minimal impedance mismatch between the specification and code.
- Denzel 10y ago
  You can eliminate the impedance mismatch entirely by using the specification as your code [1], as Alan Kay's Viewpoints research team did. ;) [1]: http://www.moserware.com/2008/04/towards-moores-law-software-part-3-of-3.html http://www.moserware.com/2008/04/towards-moores-law-software...
  - rdtsc 10y ago
    Ha! Interesting, just feed the diagram to the code.
matt_wulfeck 10y ago
It's amazing that this is still highly debatable in our industry. Always write to make debugging easier. Always. There's so many people that worship cleverness over readability.
- xelxebar 10y ago
  In all fairness, a stunningly clever insight can make things seem so obvious that you forget how hard it was to have that original insight. This kind of brevity is extremely helpful when thinking about a problem. You "just" have to grok the mechanics thoroughly. This really helps in designing effective higher level abstractions. Unequivocally admonishing brevity for ostensible readability kind of ignores the trade offs. I think there are other ways to bootstrap grokkability without having to sacrifice the conceptual clarity of nicely compact code.
  - qorrect 10y ago
    As someone taking a lot of ML/AI courses ( a subject new to me) , these clever insights are popping up all around me lately, and I agree once you learn it, it's hard to imagine what it was like before. I'm hitting a lot of 'aha' moments which has really been emotionally satisfying, it's been a long time since I had to push myself I guess.
sudshekhar 10y ago
As a side, what are some good ways to start understanding the bigger projects? Blogs and documentation are helpful but mostly don't talk about the underlying implementation details. Digging through the code line by line takes a long time and while one does learn a lot, I am sure there has to be a better way of doing this. Any static analysis tools that help in this regard? Or any other tool/approach that you might recommend?
sealjam 10y ago
I am leaving my current team, in part for this very reason. Senior technical people in the organisation don't value readable, understandable code, favouring 150 character one-liners instead. It's proven to be a very effective way of increasing the time taken for anybody except the author to debug issues. In my view, this has increased the perceived value of those responsible. I'm sure that wasn't their ultimate intention but there's certainly no incentive to change!

danbruc 10y ago

The Redis code doesn't look exceptionally good to me. I just looked through five files, but things like spacing and the use of braces seem to be pretty inconsistent. There are also a lot of magic numbers and quite a few variables have poor names.

rdtsc 10y ago
Noticed those too, also I don't personally like camelCase in C however I still like the code because of how it is laid out and solves the problem. It has the right amount of comments (too many of those is also bad), some functions are large but overall has the a good balance there too. Also for an example it is also the right sized project. It i not too small and not huge (like Linux kernel) and the overall structure can be understood in a few hours of browsing through.
bryanrasmussen 10y ago
I agree that things like spacing and inconsistent use of braces may cause code not to look good, but don't really feel it is a surefire indicator of the code not actually being good.

singularity2001 10y ago

#include "lzfP.h" # define FRST(p) (((p[0]) << 8) | p[1]) ref = hslot + LZF_HSLOT_BIAS; hslot = ip - LZF_HSLOT_BIAS; I pity all C programmers who have to deem this beautiful

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.

mapcars 10y ago

What you show in Erlang example is just a syntax. The surrounding code or usage or approach may be bad/ugly. It has nothing to do with code quality.