Hacker News new | ask | show | jobs
by itaborai83 5332 days ago
I'm curious and I might be missing more than half of my brain. Would you be willing to show some examples of bad coding on their source tree?
2 comments

I looked at using BSON in a project a while back, and ended up scrapping it mainly due to perceived poor code quality. Plenty of potential errors ignored, unclear error messages, unsafe practices.

I was also turned off by the sloppy use of memory. Heap allocated objects returned from functions with poor checks to see if anyone manages that memory on the other side. Lots of instances of strcmp, strcpy and similar unsafe string/buffer manipulation functions.

It's been a while since I looked at it so I don't have any particular examples at hand, but that was my impression.

I haven't ever used MongoDB but got interested, and first non-trivial source file I picked is this: https://github.com/mongodb/mongo/blob/master/db/btree.cpp

Take a look at for example: bool BtreeBucket<V>::find

Without even thinking about what it is doing, it's quite clear that it is not readable code, and it's not immediately obvious what the high level structure of the logic is. The function does not even fit into two screens so it's hard to reason about; your short-time memory is overused.

this is the implementation of a b+ tree. the underlying logic has been very well researched since the 70s.

if there is a part of mongodb that I am sure does not contain bugs, it is that very file you link to.

if you want to know what it does, go out and read the relevant papers on data base technology. or graduate in CS.

Clearly you didn't actually read the source file. I graduated in CS. I know B+ trees.

I also know that an 85-line, 7-argument method in a 1988-line file shouldn't depend on a global variable ("guessIncreasing") modified from several other, unrelated functions. I know that in bt_insert, which (apparently) assigns to "guessIncreasing" and then resets it to false just prior to exit, should be using an RAII class to do so instead of trying to catch every exit path, especially in a codebase that uses exceptions.

This code is amateur hour.

Thanks for attacking me personally. But I have no interest to pursue it more. I made claims that clearly hold true, and they have nothing to do with what you said (I did not say anything about bugs, for example)
That is characteristic of mathematical code, like btree. (ranty aside: being able to recognize this and find out information regarding btree for maintenance is(should be) one of the key reasons to get a CS degree)

I found the btree file relatively readable. Some macro stuff is not familiar to me, but I am sure I could figure it out in a few hours if I felt like. And I haven't yet rolled around to implementing a full-on btree, ever.