6 ms·

The first example made my skin crawl. No spacing between related blocks of code, no indentation of conditional statements to help pick them out against the back

The first example made my skin crawl. No spacing between related blocks of code, no indentation of conditional statements to help pick them out against the background, and since when is a lack of comments in the body of a method a good thing?

zakk 9y ago
> since when is a lack of comments in the body of a method a good thing? I think that's the rationale: you are supposed to explain what you do, not how you do it. The function is short enough that a global comment is enough. I agree with you, that a couple of comments in the function body wouldn't have hurt.
- zbuf 9y ago
  To be fair to the article, it 'remarks' on the lack of comments there but doesn't specifically call it out as a best practice.
w4rh4wk5 9y ago
I fully agree with you. Here is how I'd write the first example. Note that I don't know anything about SQLite and that I may have introduced typos. The comments 'resize ...' are just place holders. https://gist.github.com/W4RH4WK/57a8701190b682bcac5b8daadb43cded https://gist.github.com/W4RH4WK/57a8701190b682bcac5b8daadb43...
- zbuf 9y ago
  This is a nice example. It's not that we are not capable of understanding the original code block, but the 'breathing space' and just some minor sparse comments now allow much of that understanding in less than a few seconds -- a real practical side when working on a large body of code. And it does that without interfering with the 'good practices' highlighted in the article.
- kyberias 9y ago
  I like your formatting of the code, but... You introduce comments that are basically just noise. The name of the method already states what it does. The rest is just very simple details, actually memcpy's. Most of the comments are totally unnecessary. Code comments are not pure good, they are not Schindler's List. They are necessary evil. Every time you need to add a code comment you have failed to write clean code that it easy to understand. Every comment that you add has a cost. Someone needs to read it _in_addition_ to the code that it tries to describe. The code itself either is correct or isn't. The comment cannot fix that. But it can slow down the reader to understand what the code does. Whether there is a comment or not, the reader still needs to read the code.
  - noir_lord 9y ago
    Comments are salt, just enough enhances, too much spoils. I'll take a codebase with no comments over pages of // assign 10 to a a = 10; particularly when why you are assigning 10 would actually be a (more) useful comment.
    - w4rh4wk5 9y ago
      I support the statement that comments should explain why something is done and not how something is done, in general. See my other reply why I chose to do it like this.
  - w4rh4wk5 9y ago
    Of course you can omit the comments in the body of the function. Yet I do not think they are "just noise". In my point of view they help with two things when going over the code: First they reduce the cognitive load, second they help maintaining a red thread throughout the code. Let me explain that in more detail: while `malloc((sizeof(char) + sizeof(i16) + 1) N)` tells me how big the buffer is, it does not tell how it is organized / how it will be used. Of course you could introduce a struct for this but that would get out of hand quickly. Having the comment one line prior illustrating the layout helps one to understand what comes next. Hence offsetting `zExtra` after each `memcpy` part becomes obvious. `resize azColl` (and similar) serves as a heading for the following block, allowing one to easily find a specific step in a list of multiple steps. Scaning the whole function with your eyes for such headings is much faster than parsing for `pIdx->azColl` inside a call to `memcpy`. In the end, whether to put these kinds of comments into the code-base or not should be part of the coding convention chosen for the project. People have different opinions and after all it is a trade-off. The way I did it in this example is the way I'd do it in a big (community driven) code base like SQLite. If the convention would state otherwise, I wouldn't add them.
kyberias 9y ago
That code doesn't seem to require any comments. Code comments are usually a smell. The author couldn't write code that's easy to understand so he had to write a comment to describe it.
- cocoa19 9y ago
  I second this. For more info about why usually code comments are a code smell, I recommend reading the Clean Code book (Robert Martin), the chapter on Comments.
  - rossy 9y ago
    I've worked with people who believe this, including at least one devout follower of Clean Code and I totally disagree. One developer's obvious self-documenting code is another developer's WTF. Yeah, some projects have a lot of code that doesn't need to be commented, especially web and GUI projects that have a lot of similar code for performing CRUD operations on various database objects. That shouldn't make you get out of the habit of commenting things that actually need to be commented. One major WTF in this code that stood out for me is the place where char zExtra is cast to const char * * . (Pointer-to-pointer casts should be alarming for any C programmer.) Only later, I realised that zExtra was a block of memory containing three arrays of different type. There should at least be a brief comment explaining the memory format of zExtra, and maybe one explaining why the code makes a single allocation instead of three separate allocations (is it performance critical?)
    - kyberias 9y ago
      I don't believe you _totally_ disagree. You must understand that littering every single like of code with comments describing minute details is unnecessary. There must be a balance. I'm not a purist by any means. I don't think comments are pure evil either. They have their uses but to say that code without comments is Clean Code principles only state that code comments are not good _per se_. Most comments are just noise. Yet they are sometimes needed and bring value. It's not black and white.
logicallee 9y ago
One of the most influential few paragraphs I've ever read in my life was this FAQ entry: https://sqlite.org/faq.html#q17 https://sqlite.org/faq.html#q17 It must not be left out of any discussions of style in the SQlite codebase. Without this approach, many stylistic choices would not be nearly as acceptable. (But one thing I never looked into: do the developers use a more readable, better-commented and -spaced style, fewer tricks that produce fewer warnings, to write their test suites? Or are they written in the same style?)
- Joky 9y ago
  Testing in sqlite is great! That still isn't a reason to write unreadable code though. It may just be a "Linux Torvald" kind of approach: if you can't read the code as is we don't want you in the community.
  - logicallee 9y ago
    Do you know the answer to my question? Is the testing code (which I can only assume is not itself tested) is written in the same style and way as the code it is testing? Or is it written in a different style?
__s 9y ago
Here, have a window manager I wrote, maybe it'll make your skin crawl right off: https://github.com/serprex/nobox/blob/master/nobox.c https://github.com/serprex/nobox/blob/master/nobox.c
- marvy 9y ago
  That is COMPACT. I always thought that a window manager was supposed to be big. This fits in one file! (A note on code style though: some of your lines are a bit too long. Have pity on those with small screens. Seriously, it's not that hard to break up line 94.) Where can I learn enough about X to read this code?
  - WalterGR 9y ago
    Have pity on those with small screens. How small are your screen(s)? What resolution do you run them at? What size font? Do you have eyesight issues? I have decent vision at monitor distance. On a 13-inch Macbook Pro, using a font of reasonable size, I can easily diff two files with a line length of 108 - side-by-side. I'm curious, then, why others can't do that. (Eyesight issues aside, of course.) (Whether code lines should be 108 characters long is a separate issue.)
    - marvy 9y ago
      At home, my desktop has a resolution of 1366x768. (I usually use my laptop, which is higher than that, but whatever.) At work, I have monitors in the "portrait" position, so I only have 1080 horizontal pixels unless I'm willing to split something across two screens. (Never!) But in this case, all that is not relevant. The link was to Github, and Github enforces a width of less than a thousand pixels, regardless of screen size.
      - WalterGR 9y ago
        I have monitors in the "portrait" position Excellent, thanks. That's something I hadn't considered. But in this case, all that is not relevant. The link was to Github, and Github enforces... Ah. I assumed you were speaking generally.
userbinator 9y ago
I actually found it a welcome and refreshing change from the excessively verbose and spread-out style that seems to have become popular these days (of which Java and C# are the main examples.) I'm not sure all the casts are necessary, however, and I'd probably reduce usage of some sizeofs just using the constant (sizeof(i16) should obviously be 2, if the name is any indication.)
- kardos 9y ago
  > I'd probably reduce usage of some sizeofs just using the constant Why? The sizeof is resolved into a constant at compile time, ie, no runtime cost. I see the 'sizeof(i16)' as conveying more information than a '2'.
- aleksei 9y ago
  i16 seems to be a typedef for short int, which may be actually be larger than 2 bytes depending on the compiler.
  - minipci1321 9y ago
    Even more so: -- the fact that the code didn't use the int16_t from stddef.h but rather defined their own, could hint in the sense that a signed-int-over-exactly-16-bits might be unavailable in the ABI. People defining their own exact-width integral types (instead of pulling in stddef.h), rarely think about how that makes the code harder to interpret for a reader. Now, if the type i16 might indeed be wider than 16 bits, sizeof becomes perfectly justified but '16' is not.
  - ivanbakel 9y ago
    If it's specified as i16, it would be exactly two bytes in all situations - there would be no logic in including a size parameter that didn't actually mean anything, and sized ints are specifically meant to be platform-independent.
    - makapuf 9y ago
      But then why not use stdint ?
NicoJuicy 9y ago
You should read 'Clean Code', comments mean a failure of clean understanding code
- renox 9y ago
  And you should avoid this kind of strong assertion: there are many cases where comment are useful.. To fix a bug, I replaced a binary serialiser by a text serialiser, how could the code explain this? Also with threading, comments are often useful.
  - NicoJuicy 9y ago
    In c# , an obsolete attribute on the binaryserialiser, in the same namespace as the text serializer. With the reason why you switched and why it's obsolete now Perhaps should have added a 'mostly' in my original reply though. But I want to keep comments always to a minimum
entelechy0 9y ago
If you need to comment in the body of a function itself (there is a difference between functions and methods), then your code isn't well-written. You should be able to read the code itself and know what is going on - otherwise, it is bad code. As far as not-indenting the conditional statement bodies: That is a stylistic choice available to C programmers. If a conditional body is only 1-line, we can make the entire conditional a 1-liner, effectively reducing our total LOC-count and making fundamental understanding of what the code is doing much easier to understand. Whitespace kills

zakk 9y ago

> since when is a lack of comments in the body of a method a good thing? I think that's the rationale: you are supposed to explain what you do, not how you do it. The function is short enough that a global comment is enough. I agree with you, that a couple of comments in the function body wouldn't have hurt.

zbuf 9y ago
To be fair to the article, it 'remarks' on the lack of comments there but doesn't specifically call it out as a best practice.

w4rh4wk5 9y ago

I fully agree with you. Here is how I'd write the first example. Note that I don't know anything about SQLite and that I may have introduced typos. The comments 'resize ...' are just place holders. https://gist.github.com/W4RH4WK/57a8701190b682bcac5b8daadb43cded https://gist.github.com/W4RH4WK/57a8701190b682bcac5b8daadb43...

zbuf 9y ago
This is a nice example. It's not that we are not capable of understanding the original code block, but the 'breathing space' and just some minor sparse comments now allow much of that understanding in less than a few seconds -- a real practical side when working on a large body of code. And it does that without interfering with the 'good practices' highlighted in the article.
kyberias 9y ago
I like your formatting of the code, but... You introduce comments that are basically just noise. The name of the method already states what it does. The rest is just very simple details, actually memcpy's. Most of the comments are totally unnecessary. Code comments are not pure good, they are not Schindler's List. They are necessary evil. Every time you need to add a code comment you have failed to write clean code that it easy to understand. Every comment that you add has a cost. Someone needs to read it _in_addition_ to the code that it tries to describe. The code itself either is correct or isn't. The comment cannot fix that. But it can slow down the reader to understand what the code does. Whether there is a comment or not, the reader still needs to read the code.
- noir_lord 9y ago
  Comments are salt, just enough enhances, too much spoils. I'll take a codebase with no comments over pages of // assign 10 to a a = 10; particularly when why you are assigning 10 would actually be a (more) useful comment.
  - w4rh4wk5 9y ago
    I support the statement that comments should explain why something is done and not how something is done, in general. See my other reply why I chose to do it like this.
- w4rh4wk5 9y ago
  Of course you can omit the comments in the body of the function. Yet I do not think they are "just noise". In my point of view they help with two things when going over the code: First they reduce the cognitive load, second they help maintaining a red thread throughout the code. Let me explain that in more detail: while `malloc((sizeof(char) + sizeof(i16) + 1) N)` tells me how big the buffer is, it does not tell how it is organized / how it will be used. Of course you could introduce a struct for this but that would get out of hand quickly. Having the comment one line prior illustrating the layout helps one to understand what comes next. Hence offsetting `zExtra` after each `memcpy` part becomes obvious. `resize azColl` (and similar) serves as a heading for the following block, allowing one to easily find a specific step in a list of multiple steps. Scaning the whole function with your eyes for such headings is much faster than parsing for `pIdx->azColl` inside a call to `memcpy`. In the end, whether to put these kinds of comments into the code-base or not should be part of the coding convention chosen for the project. People have different opinions and after all it is a trade-off. The way I did it in this example is the way I'd do it in a big (community driven) code base like SQLite. If the convention would state otherwise, I wouldn't add them.

kyberias 9y ago

That code doesn't seem to require any comments. Code comments are usually a smell. The author couldn't write code that's easy to understand so he had to write a comment to describe it.

cocoa19 9y ago
I second this. For more info about why usually code comments are a code smell, I recommend reading the Clean Code book (Robert Martin), the chapter on Comments.
- rossy 9y ago
  I've worked with people who believe this, including at least one devout follower of Clean Code and I totally disagree. One developer's obvious self-documenting code is another developer's WTF. Yeah, some projects have a lot of code that doesn't need to be commented, especially web and GUI projects that have a lot of similar code for performing CRUD operations on various database objects. That shouldn't make you get out of the habit of commenting things that actually need to be commented. One major WTF in this code that stood out for me is the place where char zExtra is cast to const char * * . (Pointer-to-pointer casts should be alarming for any C programmer.) Only later, I realised that zExtra was a block of memory containing three arrays of different type. There should at least be a brief comment explaining the memory format of zExtra, and maybe one explaining why the code makes a single allocation instead of three separate allocations (is it performance critical?)
  - kyberias 9y ago
    I don't believe you _totally_ disagree. You must understand that littering every single like of code with comments describing minute details is unnecessary. There must be a balance. I'm not a purist by any means. I don't think comments are pure evil either. They have their uses but to say that code without comments is Clean Code principles only state that code comments are not good _per se_. Most comments are just noise. Yet they are sometimes needed and bring value. It's not black and white.

logicallee 9y ago

One of the most influential few paragraphs I've ever read in my life was this FAQ entry: https://sqlite.org/faq.html#q17 https://sqlite.org/faq.html#q17 It must not be left out of any discussions of style in the SQlite codebase. Without this approach, many stylistic choices would not be nearly as acceptable. (But one thing I never looked into: do the developers use a more readable, better-commented and -spaced style, fewer tricks that produce fewer warnings, to write their test suites? Or are they written in the same style?)

Joky 9y ago
Testing in sqlite is great! That still isn't a reason to write unreadable code though. It may just be a "Linux Torvald" kind of approach: if you can't read the code as is we don't want you in the community.
- logicallee 9y ago
  Do you know the answer to my question? Is the testing code (which I can only assume is not itself tested) is written in the same style and way as the code it is testing? Or is it written in a different style?

__s 9y ago

Here, have a window manager I wrote, maybe it'll make your skin crawl right off: https://github.com/serprex/nobox/blob/master/nobox.c https://github.com/serprex/nobox/blob/master/nobox.c

marvy 9y ago
That is COMPACT. I always thought that a window manager was supposed to be big. This fits in one file! (A note on code style though: some of your lines are a bit too long. Have pity on those with small screens. Seriously, it's not that hard to break up line 94.) Where can I learn enough about X to read this code?
- WalterGR 9y ago
  Have pity on those with small screens. How small are your screen(s)? What resolution do you run them at? What size font? Do you have eyesight issues? I have decent vision at monitor distance. On a 13-inch Macbook Pro, using a font of reasonable size, I can easily diff two files with a line length of 108 - side-by-side. I'm curious, then, why others can't do that. (Eyesight issues aside, of course.) (Whether code lines should be 108 characters long is a separate issue.)
  - marvy 9y ago
    At home, my desktop has a resolution of 1366x768. (I usually use my laptop, which is higher than that, but whatever.) At work, I have monitors in the "portrait" position, so I only have 1080 horizontal pixels unless I'm willing to split something across two screens. (Never!) But in this case, all that is not relevant. The link was to Github, and Github enforces a width of less than a thousand pixels, regardless of screen size.
    - WalterGR 9y ago
      I have monitors in the "portrait" position Excellent, thanks. That's something I hadn't considered. But in this case, all that is not relevant. The link was to Github, and Github enforces... Ah. I assumed you were speaking generally.

userbinator 9y ago

I actually found it a welcome and refreshing change from the excessively verbose and spread-out style that seems to have become popular these days (of which Java and C# are the main examples.) I'm not sure all the casts are necessary, however, and I'd probably reduce usage of some sizeofs just using the constant (sizeof(i16) should obviously be 2, if the name is any indication.)

kardos 9y ago
> I'd probably reduce usage of some sizeofs just using the constant Why? The sizeof is resolved into a constant at compile time, ie, no runtime cost. I see the 'sizeof(i16)' as conveying more information than a '2'.
aleksei 9y ago
i16 seems to be a typedef for short int, which may be actually be larger than 2 bytes depending on the compiler.
- minipci1321 9y ago
  Even more so: -- the fact that the code didn't use the int16_t from stddef.h but rather defined their own, could hint in the sense that a signed-int-over-exactly-16-bits might be unavailable in the ABI. People defining their own exact-width integral types (instead of pulling in stddef.h), rarely think about how that makes the code harder to interpret for a reader. Now, if the type i16 might indeed be wider than 16 bits, sizeof becomes perfectly justified but '16' is not.
- ivanbakel 9y ago
  If it's specified as i16, it would be exactly two bytes in all situations - there would be no logic in including a size parameter that didn't actually mean anything, and sized ints are specifically meant to be platform-independent.
  - makapuf 9y ago
    But then why not use stdint ?

NicoJuicy 9y ago

You should read 'Clean Code', comments mean a failure of clean understanding code

renox 9y ago
And you should avoid this kind of strong assertion: there are many cases where comment are useful.. To fix a bug, I replaced a binary serialiser by a text serialiser, how could the code explain this? Also with threading, comments are often useful.
- NicoJuicy 9y ago
  In c# , an obsolete attribute on the binaryserialiser, in the same namespace as the text serializer. With the reason why you switched and why it's obsolete now Perhaps should have added a 'mostly' in my original reply though. But I want to keep comments always to a minimum

entelechy0 9y ago

If you need to comment in the body of a function itself (there is a difference between functions and methods), then your code isn't well-written. You should be able to read the code itself and know what is going on - otherwise, it is bad code. As far as not-indenting the conditional statement bodies: That is a stylistic choice available to C programmers. If a conditional body is only 1-line, we can make the entire conditional a 1-liner, effectively reducing our total LOC-count and making fundamental understanding of what the code is doing much easier to understand. Whitespace kills