8 ms·

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.

by pilif 10y ago

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.

johncolanduoni 10y ago
> 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.
- pilif 10y ago
  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.
  - deleted 10y ago
    [deleted]
  - msvalkon 10y ago
    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.
    - throwaway2048 10y ago
      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.
      - rimantas 10y ago
        Exactly. Not all code can be understandable to layman with zero effort.
    - dang 10y ago
      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.
- vertex-four 10y ago
  Which bits of the Rust runtime(?) do you think are too high overhead for this?
  - chillydawg 10y ago
    I suspect any runtime, at all, would be too much overhead.
    - vertex-four 10y ago
      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.
      - johncolanduoni 10y ago
        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.
    - jdub 10y ago
      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.
      - johncolanduoni 10y ago
        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.
  - johncolanduoni 10y ago
    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.
- deleted 10y ago
  [deleted]
- 72deluxe 10y ago
  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).
- PeCaN 10y ago
  > 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.
nicky0 10y ago
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...)
- B1FF_PSUVM 10y ago
  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.
  - johncolanduoni 10y ago
    Or maybe they think it'll just help other people (including future you) read the code? Like was claimed?
- pilif 10y ago
  > 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.
  - nicky0 10y ago
MichaelBurge 10y ago
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.
- rimantas 10y ago
  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.
verytrivial 10y ago
It's 2016 and it on github. Fix it yourself!
benkarst 10y ago
They open sourced it for you to clean up!
userbinator 10y ago
"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
- douche 10y ago
  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.
  - gutnor 10y ago
    > 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.
eru 10y ago
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.)
satysin 10y ago
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.
dang 10y ago
We detached this subthread from https://news.ycombinator.com/item?id=12047902 https://news.ycombinator.com/item?id=12047902 and marked it off-topic.

by pilif 10y ago

johncolanduoni 10y ago

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

pilif 10y ago
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.
- deleted 10y ago
  [deleted]
- msvalkon 10y ago
  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.
  - throwaway2048 10y ago
    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.
    - rimantas 10y ago
      Exactly. Not all code can be understandable to layman with zero effort.
  - dang 10y ago
    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.
vertex-four 10y ago
Which bits of the Rust runtime(?) do you think are too high overhead for this?
- chillydawg 10y ago
  I suspect any runtime, at all, would be too much overhead.
  - vertex-four 10y ago
    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.
    - johncolanduoni 10y ago
      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.
  - jdub 10y ago
    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.
    - johncolanduoni 10y ago
      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.
- johncolanduoni 10y ago
  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.
deleted 10y ago
[deleted]
72deluxe 10y ago
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).
PeCaN 10y ago
> 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.

nicky0 10y ago

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

B1FF_PSUVM 10y ago
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.
- johncolanduoni 10y ago
  Or maybe they think it'll just help other people (including future you) read the code? Like was claimed?
pilif 10y ago
> 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.
- nicky0 10y ago

MichaelBurge 10y ago

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.

rimantas 10y ago
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.

verytrivial 10y ago

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

benkarst 10y ago

They open sourced it for you to clean up!

userbinator 10y ago

"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

douche 10y ago
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.
- gutnor 10y ago
  > 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.

eru 10y ago

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

satysin 10y ago

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.

dang 10y ago

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