Comment by rdtsc | Hacker News Reader

Comment by rdtsc | Hacker News Reader

Not Ruby but for C code, I like Redis's code:

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

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.

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.

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.

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.

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.

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 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.

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...

Ha! Interesting, just feed the diagram to the code.

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?

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!

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.

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.

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.

#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

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.

> 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.

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.

A URL to the actual source helps, I think, to put it in context: 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.

This is perfectly fine and readable. No need to pity anyone, especially not C programmers. :)

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.

Reply on news.ycombinator.com