Elevate your C programming skills by exploring the SQLite codebase

by cpprepository • 9 years ago

105 73 9 years ago

Philipp__• 9 years ago

But reading that amalgamation sqlite3.c file wasn't the best C reading-for-learning experience I had.

I found Redis and OpenBSD much more organized and documented, it just made more sense since the structure of those projects is really nicely organized. Of course there are reason why sqlite3 is one C file, I am well aware of that, but I just wanted to point out 2 more awesome sources for reading and learning C.

ccrush• 9 years ago

IIRC, the original source is split into many files and you can build a one file version, and the reasoning behind doing so has to do with compile and link performance.

agumonkey• 9 years ago

One of my favorite C reading experience is maru http://piumarta.com/software/maru/

(self bootstrapping lisp)

ivanbakel2• 9 years ago

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• 9 years 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.

zbuf2• 9 years 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.

w4rh4wk54• 9 years 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/57a8701190b682bcac5b8daadb43...

zbuf2• 9 years 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.

kyberias5• 9 years 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.

kyberias5• 9 years 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• 9 years 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.

logicallee2• 9 years 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

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• 9 years 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.

__s2• 9 years 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

marvy4• 9 years 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?

userbinator• 9 years 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• 9 years 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• 9 years ago

i16 seems to be a typedef for short int, which may be actually be larger than 2 bytes depending on the compiler.

NicoJuicy2• 9 years ago

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

renox• 9 years 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.

SonOfLilit• 9 years ago

I feel like TFA makes a very bad case for it (they use structs? only 8 functions have many parameters? your commercial tool can query this kind of thing with SQL?), but regardless, SQLite is a very good codebase to delve into as a seasoned programmer wanting to improve even more.

What worked for me was diving in with a specific question (I think it was "I wonder how indexes are represented on disk?") until I found the answer, picking up a lot on the way.

Not an easy codebase to read, but full of good architecture, good ideas and best practices.

vmp• 9 years ago

The function body is well indented

To be fair, there is only one level of indentation. Kind of difficult to screw that up.

omginternets• 9 years ago

Is it me or is the scrolling behavior on this site awful?

ThePadawan• 9 years ago

Same here (on a very high-DPI display). Apparently smoothscroll.js is responsible.

photonios• 9 years ago

Yeap. I wanted to swipe back on the trackpad of my backbook and it just scrolled down. Not ok.

minipci13212• 9 years ago

When the 'return' statement is put on the same line as the parent 'if' statement, setting a breakpoint on returning requires dealing with the assembly code and hex values (identifying the instruction that implements the return (could be a ret or a jump), then using its address to set the breakpoint.

When the return statement is kept on its own line, this is much easier.

pjmlp4• 9 years ago

Or using advanced GUI debuggers that allow middle statement breakpoints.

to3m2• 9 years ago

Which debuggers for C or C++ do this, and which compilers do they work with?

jankotek• 9 years ago

For Java I would recommend to study H2 SQL database. It is small self-contained project, with well written and consistent code.

jstimpfle2• 9 years ago

Const-correctness is really difficult to get right. One person's const is another person's mutable.

An example from the top of my head, a function which finds an integer in an array "should" get that array as a "const int *". But if the caller wants to modify the found element, it needs to do ugly casting (really bad practice).

Similar problems exist in most other programming languages. Most have only "shallow" const. You can mark an Java array member as final, but you can't disallow mutations to that array.

Const-correctness is similar to types - it breaks very quickly, and when it breaks it gets in the way.

That means I resort to indices when possible. When pointers are needed I typically avoid const.

pif• 9 years ago

a function which finds an integer in an array "should" get that array as a "const int ". But if the caller wants to modify the found element, it needs to do ugly casting (really bad practice).

My friend, your example is wrong! The caller can happily work with a mutable array; the function accepting a "const" pointer is just a guarantee that the function itself* won't modify the content.

  size_t look4 (const int *ptr, int val);
  void caller (int *ptr, int old_val, int new_val)
  {
      size_t index = look4(ptr, old_val);
      ptr[index] = new_val;
  }

pjmlp4• 9 years ago

I know you are just giving an example, but caller() should check that what came from look4() is a valid index into ptr.

jjnoakes• 9 years ago

Sure, if you return an index. I bet the parent poster was discussing returning a pointer, though.

Imagine look4 returned a pointer to the found element. Making look4's signature const-correct in C is not possible.

(Think of strchr).

jstimpfle2• 9 years ago

As my sibling poster explained, that's not why meant.

Try to reduce some of the problems these people have: https://stackoverflow.com/questions/856542/elegant-solution-...

sidlls2• 9 years ago

It is a nice/ok code base. I find Redis' a bit better.

One thing that stood out in the article was calling out the percentage of functions with fewer than X lines. I don't think that is a good goal to work toward. Having functions that do one thing and do it well is, and function length can be a very rough proxy measure for that. However "doing one thing" almost always means "perform more than one step in an algorithm." I've seen too many code bases where "keep functions short" is used as an excuse to break locality by piecing functions out into smaller functions that are only ever called in one place (the function they split up). It can make code harder to read and maintain and be less efficient besides.

chubot3• 9 years ago

Eh, IMO it depends on how many locals there are more than the number of lines. Reducing the state space definitely improves readability.

If you have a function with 15 locals, it could be hard to keep straight in your head, even if that function were 50 lines or so. If you can split it up into 3 functions with 5 locals each (and a small number of arguments), that is usually more readable.

As long as the functions are named well and communicate (only) with params and return values, it doesn't hurt readability to split it up, even if it's only used in one place.

On the other hand, I suppose you can have a 500 line function and if has few locals, then you probably don't need to split it up. The canonical example would be a function that only prints stuff to the screen, maybe based on a couple parameters. But I don't really run into those situations very much -- more often I run into the big functions that can be split up.

saurik• 9 years ago

I don't know. My absolutely favorite codebase ever was dpkg. It isn't as good now as it was before, but essentially they arranged so that anything "unusual" (error handling and the unwind process) happened in a side channel via setjmp and then the core of the program was in a file called processarc.c which contained one giant function that you could use to see, in order, all of the middly steps that dpkg performed. Doing it like that meant that the code read like a specification.

I am highly skeptical of codebases with lots of functions that are called from only one place; I would even put that forward, along with "code duplication (not just in the small but also structurally: where you should be building higher-order primitives, including your own control structures or scaffold) as its foil (as it usually isn't OK to not have a function if it would be called twice), as core metrics for "bad code". If you have too many locals... use a block! That's what they are for :/.

eru4• 9 years ago

As long as the functions are named well and communicate (only) with params and return values, it doesn't hurt readability to split it up, even if it's only used in one place.

It's interesting how even in the context of C the virtues that functional programming dials up to eleves come up.

(Git is similar: it's a dirty C program with lots of mutation under the hood, but when you use it you can sort-of pretend it purely functional, since the main things you care about are append-only, and even rewriting history just creates a new slightly altered copy of history, and the old one is available until the GC kicks in. Similar for the recent craze of copy-on-write in filesystems.)

eru4• 9 years ago

One point to complement your argument:

Breaking out a function that's only ever called once can still be a good idea: functions---especially pure ones---communicate with their environment via narrow interfaces, their arguments and return values.

Compare that to inlining the same block of code: the reader will have to carefully check that the inlined block is self-contained, instead of having the syntax provide the hint.

Granted, this argument mostly makes sense for pure functions, and doesn't apply nearly as often in C as in eg OCaml. Haskell's usual style is perhaps the most extreme in that regard, any individual definition only very seldom goes on for longer than one screen. (Forth people strive for a similar style of ravioli code, but I don't have the experience with that language to say much.)

wav-part2• 9 years ago

While we are talking about C skills, recently I got hit by this gotcha. I never knew a simple comparision could be this complex.

Question: Whats ru, rs ?

    unsigned int u0 = 0;
    unsigned int u1 = 1;
    int s0 = 0;
    int s1 = 1;
    int ru = 0 > u0 - u1;
    int rs = 0 > s0 - s1;

Answer: ru is 0. rs is 1.

jonsen• 9 years ago

I wouldn't say it is complex, that we have unsigned comparison for unsigned numbers, and signed comparison for signed numbers.

MaulingMonkey• 9 years ago

For this specific case, warnings can save you:

https://wandbox.org/permlink/wwbCYysH039z3YqN

  prog.cc: In function 'int main()':
  prog.cc:8:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
       int ru = 0 > u0 - u1;
                ~~^~~~~~~~~

wav-part2• 9 years ago

I had -Wall set but I forgot that its misleading. -Wextra is needed for -Wtype-limits and -Wsign-compare.

kerneldeveloper• 9 years ago

I think the Linux kernel source code is also very clean and it's worth reading. I prefer its coding style.

CalChris• 9 years ago

This strikes me as programming like it's 1999 (1998 actually). It's not the worst by far. But then I wouldn't call it an elevation either.

1. Elegant use of white space makes for an easier read.

2. Intermingled declarations and code showed up in C99.

3. C++ style comments rather than block comments. There should be a commentary (and not a recital).

4. camelCase for local variables? Really?

5. Single line if statements make for a hard read.

Elevate your C programming skills by exploring the SQLite codebase

Related Stories