Hacker News Reader

Comment by pilif | Hacker News Reader

It's 2016. How can you launch a reasonably high profile open source project with code that looks like this? This fulfills all the TODO list for unreadable code. One character variable names, one character parameter names, full of magic numbers...

Yes. This is very performance critical code and I completely see the need to write very optimized code. That's fine. But optimizing code for speed shouldn't imply also optimizing it to use as few characters as possible.

Compression code is code that often runs at boundaries to the external world and thus is a very significant attack surface. To release compression code in a non-safe language is risky enough but then using what amounts to practically write-only code is, IMHO, irresponsible.

View on HN

We detached this subthread from https://news.ycombinator.com/item?id=12047902 and marked it off-topic.

"It's 2016." So what? Have people lost the ability to use abbreviations? One letter is perfectly fine because they are abbreviations, the purpose of which you should recognise immediately if you understand the tiniest bit of what LZ algorithms do and the concepts surrounding them. D = distance, L = long-distance, M = medium-distance.

You may ask, "Why and what is q"? By only looking at the fragment posted, where q is the output pointer, I can already guess there is an input pointer named p, and glancing at the full file shows that is indeed the case. x is also a temporary. This is a very common convention.

Sadly, I'm increasingly finding that code these days is some bloated monstrosity with variable names that barely fit into 80 columns and plenty of ridiculous indirection that turns what really needs only a single line into a deep function-call-chain spanning dozens of lines (not all in the same place) and maybe even across multiple files. Reading "modern" C# and Java code makes my head hurt with all the verbiage --- there is so much code, but very little actual substance.

This code is essentially all substance and little verbiage, and I can comprehend it quite easily. Thus I find the style complaints entirely unfounded.

To borrow a sentiment from Linus Torvalds: the code is "unreadable" to you, because you are not (yet) qualified to read it.

/rant

It's not like you have to pay a dime for every character in your source code. With halfway-decent autocomplete, longer identifiers are easier to use than shorter ones, I've found.

I'd hope you'd at least put a comment header to explain what the parameters actually are for anyone who doesn't have the paper handy, or god forbid, used a paper implementing the same thing using different nomenclature.

> for anyone who doesn't have the paper handy

You are supposed to have the paper handy. Either you know the paper by heart, or you are actually learning the paper and look at the implementation. Otherwise you really have nothing to do in this piece of code.

Even more mundane codebase are like that. Variable are named in the context of the project. If you have no familiarity with the project variable "user" or "u" means exactly the same to you: nothing.

The difference is that generally regular project are huge in size but simple, while filesystem, compression/encryption algorightm, trading algorithm are tiny but extremely complex. In the former, you use more descriptive naming because people will only have a high level knowledge of the spec. In the later, there is no difference between spec and code, extreme familiarity is necessary to touch the code and naming convention crutches are simply unnecessary.

For math it's the other way around: long identifiers make things harder to read. When reading the code, the pattern of operators is what really matters; variables happen to be plugged into them.

For example, any person with a modicum of physics background will recognize the following as a kinetic energy calculation, even if I use random letters for the variables.

   X = (a * b^2)/2

But if I throw that in a codebase for some web project, I would use fully explicit variable names.

This falls under the category of "know the audience for whom you are writing code".

For example, any person with a modicum of physics background will recognize the following as a kinetic energy calculation, even if I use random letters for the variables.

Someone with a physics background might assume that X is the kinetic energy of an object with mass a and velocity b. Or they might assume that X is the displacement of object after having acceleration a for a time b, having initially been at rest. The latter is perhaps more reasonable, since then the choice of two of three three variable names (x and a) is conventional.

A reason why single-letter variable names are practical is that there are strong conventions about what particular variables might represent: eg. start of the roman alphabet is constants, end of the roman alphabet is variables, capital letters are matrices, many letters in the roman and Greek alphabets have one (or a few) common meanings.

You could offer to rewrite if for them...

It's an implementation of a mathematical algorithm. It doesn't need allTheVariables toBeNamed likeThis. Single letters map to meaningful concepts in the mathematical algorithm.

I don't see how giving the variables longer names would make it more readable. Indeed I think long variable names would obscure the structure.

Code like this has to be looked at in the concept of the algorithm design (which I hope exists...)

It's funny, because there's a whole couple of generations out there that have been taught that explicitVariableNames will fix their thinking deficit.

I mean, if it weren't sad.

Or maybe they think it'll just help other people (including future you) read the code? Like was claimed?

> You could offer to rewrite if for them...

Nope. Because I can't read it. I plainly do not have the information needed to understand what's going on here, nor is the design document part of the source code. As you seem to have no trouble understanding what's going on, can you enlighten me, for example, what's so special about the number 271 that we see M being compared to?

> I don't see how giving the variables longer names would make it more readable.

Variables that are part of the algorithm itself probably make sense in their short name (though that's a common problem with math in general. Being more expressive isn't a bad thing), but there are also parameters to public API that are equally short for no gain.

Also, the code is full of magic numbers and I'm sure an algorithm design document would at least give names out to them.

Besides, even if the document was available (it isn't, at least not to the public), there's absolutely zero harm in making the code more accessible or at least reference the spec.

OK but which is more readable ... (I know it's a bit silly, I juts made up names)

} else if (D >= (1 << 14) || M == 0 || (x + 3) + M > 34) {

} else if (DirectWeightingFactor >= HUYGENS_LIMIT || MariachiBand == 0 || (xylemNonce + STANDARD_PZSH_INCREMENT) + MariachiBand > SWIM_RATE_B) {

I guess your opinion differs to mine. I like the one that looks like math.

What about a middle ground?

} else if (D >= HUYGENS_LIMIT || M == 0 || (x+3)+M > SWIM_RATE_B)

And then somebody else will write angry hackernews comment how code is abysmal because it mixes in single char names. ;)

The more verbose name is infinitely more preferable to me. If I am tasked one day with doing some maintenance on this code and have never encountered it then at least I have a hint that I should be researching the aquatic properties of mariachi bands.

I would have a hard time with that, the long names hide the structure. Know your audience. Do you expect future maintainers for your compression algorithm to be people with knowledge of compression algorithms? Or do you expect them to be random programmers off the street?

As someone who's written and maintained code like this, in my experience the heavy algorithmic parts themselves usually requires almost no maintenance. Its the surrounding code - the API endpoints, makefiles and test suites that need to be modernized every few years. Algorithms like this are written, then used, then better algorithms are written from scratch.

Code like this also usually can't be edited piecemeal. Any change requires reloading the entire algorithm into your head before a single line is changed. And as others have mentioned, the normal way to do that is to read the paper. .. And if you do that, the paper's naming convention becomes the natural way to express the algorithm.

I'd like the one where this whole thing is put in a function with a name describing what it does, no matter whether the math-like one or the one with long names is used. that I'd consider readable. And any sane compiler inlines it anyway.

> what's so special about the number 271?

It's a Cuban Prime. Everybody knows that. Geez!

271 is the longest length possible (the code is writing out some kind of literal data). The count, -16, follows each 0xE0 byte (see emit_literal - recall Apple only use little-endian CPUs). 255+16=271.

When the byte is 0xE1...0xEF (inclusive), the bottom 4 bits alone are the count - so after a run of 0xE0 sections this is how you write out any stragglers.

The decoder has comments, but I managed to figure the above out without them, so I doubt it can be that hard - I've just had a fair amount of practice at this bit twiddling C stuff. At some point when writing code you have to assume that the reader will be speaking your language.

https://github.com/lzfse/lzfse/blob/master/src/lzvn_decode_b...

> I plainly do not have the information needed to understand what's going on here

That's not a problem with the code, that's you not understanding the math that's actually happening. For what it's worth, neither do I, but from what I'm gleaning from other comments here, it's a C implementation of a mathematical proof, so it'd be better to go learn the math than expect to be given friendly code. Besides, "open source" doesn't mean "catered to the lowest common denominator" - they don't write code for you or I to understand without some work, we have to earn the knowledge.

I feel like spitting on other people's code is a good way to become an unhappy person. And this is coming from someone who's had a few head-scratching gdb sessions when a segfault occasionally appears in similar LZF compression code.

And that's because doing the work is more valuable than being a snoot in the style aristocracy.

We have a proverb/parble which goes like this:

  —Lazy-bones, lazy-bones, would you like a boiled egg?
  —Is it peeled off?
  —Nope.
  —Throw it away.

> To release compression code in a non-safe language is risky enough

At the moment, what's their real alternative? Rust is the only memory-safe language I can think of that could hope to meet their performance requirements, but even the Rust runtime would be a lot of overhead for this application.

That said, I agree this isn't acceptable C code for something that runs on untrusted data while using tons of pointer arithmetic.

You're right about C. C in general, I would find acceptable, because, yes, there aren't that many good alternatives around for this kind of code.

But there's nothing stopping you from writing readable C code. That's where my concerns come from.

I don't really understand where the downvotes come from? I find the readability concerns legitimate, and would like to understand why compression algorithm developers feel like this is OK? Is it just the math heavy background? Can't think of any real benefits to this style.

If you don't understand the underlying mathematical algorithms its using, no amount of explicit varible names are going to help you. If you do, the concise structure makes things straightforward. The code is not meant to be read alone and understood, the papers published along with it need to be understood first.

Exactly. Not all code can be understandable to layman with zero effort.

I suspect the downvotes are because code readability is a complex, subtle topic that often gets reduced to flamewars by people who are sure they know ‘the’ right way to do things.

Code readability is relative to the reader, the programming language, and the conventions of a codebase. That's a lot of things to be relative to! Knowing that ought to put speed bumps on the way to dismissing code one isn't familiar with.

I remember having a reaction years ago on seeing some of P.J. Plauger's C++ standard library code. I think I burst out laughing and said I'd fire anyone who wrote code like that for me. Years of subsequent experience have brought multiple layers of understanding how wrong I was.

Which bits of the Rust runtime(?) do you think are too high overhead for this?

I suspect any runtime, at all, would be too much overhead.

There isn't a Rust runtime, though, in the sense that you seem to be implying. There's a standard library, which is what I assumed they meant.

Yes, I was using it to describe the standard library. C and C++ are both described as having a runtime, and Rust has one in the same sense. In any case I would consider the code needed for handling stack unwinding to be worthy of the name runtime (small though it may be).

I understand it doesn't have a runtime in the way a JIT'ed or GC'ed language has a runtime, but it's a runtime nonetheless.

This C code doesn't use the C library. Code like this in Rust wouldn't engage with any part of the Rust standard library either. There's no runtime work to be done, in either language.

There is still an overhead in terms of executable size, unless you use #![no_std]. This environment is not easy to code in; it doesn't even have heap allocation.

Using rust 1.9.0, an empty (save for a function that adds two u32s) standalone dynamic library built in release mode on OS X is 1.6 MB. A static library is a whopping 2.4 MB. The comparable number for C are 4K and 800 bytes respectively.

Asking every client of the compression library to pull in that much overhead would likely make it rather unpopular. Until Rust gets better at eliminating unnecessary parts of the runtime when a program doesn't use it (something like GCC's gc-sections), it's not going to be feasible for small libraries to be written in it.

My understanding is that the majority of that is jemalloc, libbacktrace, and glibc. jemalloc can be replaced with system malloc easily, libbacktrace can be removed by setting the compiler to interpret panics as aborts (which you need to do in a library used by C anyway, really), and glibc can be replaced with musl. This can bring binary size down to about 160kb for a binary which just does printf!(). Still not quite as good as C, but a lot better than default Rust with just a few tweaks.

  > (which you need to do in a library used by C anyway, really)

You can also use panic::catch_unwind at the boundary too, it depends on what you want to do.

> A static library is a whopping 2.4 MB. The comparable number for C are 4K and 800 bytes respectively.

It is possible to significantly optimize that number [0]. Not that binary size is not an issue, but rather 2.4mb vs. 4kb is not an apples to apples comparison

[0]: https://lifthrasiir.github.io/rustlog/why-is-a-rust-executab...

The article you linked is doing all of this with a binary. Last time I tried something like this with Rust, there were a lot more obstacles to cutting down this overhead with a library than a binary. Also note that towards the bottom he cuts out libstd, which loses any form of dynamic memory allocation, as well as a significant chunk of Rust's usability advantage.

The biggest factor however is that you have to read through that whole page, use unstable features (alloc_system) that condemn you to the nightly, and download and compile musl. This is a huge, brittle pain at the moment, and far from obvious to anyone who comes upon Rust and is thinking of building a C-compatible library using it.

> as well as a significant chunk of Rust's usability advantage.

What bits are you thinking of here? Just curious, as I do a lot of no_std work, and don't feel that way, and am probably blind to it :)

Rust 1.10, coming out later today, has a new crate type that removes Rust-specific metadata for dynamic libraries, by the way, making them a bit smaller for this kind of case.

Unless I'm mistaken, no_std means no built-in non-manual dynamic allocation (Box, Rc, etc.), unless you use "extern crate alloc", once again requiring the nightly. Some fundamentals one expects from a modern language like Vec are also missing in either case.

This is fine if you're using no_std for something where these are anathema anyway (writing bare-metal OSes comes to mind) but a huge limitation for a humble user-space library. As it stands if you want to take advantage of Rust's safety you're going to need to reimplement at least Box, probably Vec, and Rc if your program requires that kind of thing. This isn't a huge time suck, but if I were feeling out C-compatible languages before writing a library it would be a major turnoff.

I really like Rust for low-overhead binaries but it is missing a lot when it comes to writing non-rlib libraries.

Ah I see. There's two things here: first off, I'm using it in an OSdev context, so I don't expect any allocation to exist, since I haven't actually implemented that yet. And second, I took your comment to mean the language itself, which doesn't lose anything with no_std, but you mean the convenience of the libraries, which makes sense.

By the way, you _can_ reintroduce just those things if you want to. no_std means "don't include std", but you can then require them:

    #![feature(alloc)]
    #![feature(collections)]
    #![no_std]

    extern crate alloc;
    extern crate collections;

    use alloc::boxed::Box;
    use alloc::rc::Rc;
    use collections::vec::Vec;

    pub fn foo() -> Box<i32> {
        Box::new(5)
    }

    pub fn bar() -> Rc<i32> {
        Rc::new(5)
    }

    pub fn baz() -> Vec<i32> {
        let mut v = Vec::new();

        v.push(5);

        v
    }

Of course, as you can see, the facade crates are largely not stable, so doing this on _stable_ rust isn't quite there yet, which is a thing that matters, as you originally pointed out. I expect as Rust grows for this stuff to stabilize, after all, the std versions are re-exported, so this example is de-facto stable, other than maybe the 'use' lines, which is an easy fix in the future.

Thanks for elaborating :)

Yes, my issue is just that creating a small library with a C API requires a lot of machinations that are going to turn off anyone who isn't really set on going with Rust. For OS development, standalone binaries, or Rust libraries, I think Rust is in excellent shape as it is.

May I ask what is not memory safe about C++ in this situation? I am talking C++11, not the widespread C++98 stuff we find everywhere.

Or why not use local variables that are guaranteed to be cleaned up?

I suspect Apple would have an alternative to write this in Swift, but that would probably have speed implications (I am guessing).

> At the moment, what's their real alternative?

Ada? Chapel? ATS? D if you avoid the GC?

> That said, I agree this isn't acceptable C code for something that runs on untrusted data while using tons of pointer arithmetic.

Why not? It's not the prettiest code ever, but it gets the point across of what's going on.

I agree on the C-bashing. But the single-letter variable names are fine---they correspond to the paper, and are probably very mathematical entities that don't even have real longer names.

(Longer names are for book keeping, not so much for calculation.)

I would agree if this were a "normal" project but it isn't. It is a compression algorithm which is essentially an implementation of a mathematical proof wrapped in some I/O functions.

It's 2016 and it on github. Fix it yourself!

They open sourced it for you to clean up!

Reply on news.ycombinator.com