The tone of this article is very rude and immature. It sounds more like the author is trying to shame the “Bitcoin Unlimited” people for some reason more than he’s really trying to be helpful.
I agree it is rude, but I also feel with author. This is literally code, which wants to handle billions and trillions of dollars worth of Bitcoins. Misusing assert is not something which gives me confidence.
If those were the biggest gripes he could find over the whole code base, then I’d say the code stands up really well actually.
At least two of the issues are ugly but harmless (checking the result of new and the one that checks a big condition and then asserts false). One of them is a non-issue if asserts are turned on in release builds (there’s a compile flag, are they using it?).
The remaining one, using assert to check if data is on disk, is not how I’d use assert, but I can see how it’s justified if the code should really never run when the data isn’t there and there’s no way to recover. I don’t know enough about the codebase and how the system works to judge one way or the other, so I’ll give them the benefit of the doubt.
“These people don’t write code exactly the way I do, therefore they are bad coders,” is not a valid argument, and there’s really not a standard agreed upon rule on how to use assert.
On the contrary, if the code is never compiled with NDEBUG defined then view.Flush() will always get called. For better or worse, lots of applications ship with asserts turned on, even in release builds.
I’m not saying it’s a great use of assert, but it’s also not so bad that the developer deserves to be shamed with an article calling him out as a bad coder, either.
The author of the article is pointing out genuine misuse of assert() in the program. Asserts are used explicitly to catch “impossible” program states, thus helping the debugging and development process. They are meant to be redundant. If an assertion fails, it shows that you have a bug in your code. The fact that the original authors of the code are using these as a way to check user input, as well as check if something that has side effects has succeeded, shows that they have a fundamental misunderstanding of what asserts are actually used for.
Assert is disabled with NDEBUG. The Debug vs Release issue the author is talking about seems more like an IDE thing.
This is more than an “IDE thing”. Programs depend on these compile-time definitions to include or exclude special debug information - like asserts or compile-time warnings. This source was abusing the use of asserts, which as a side effect, made DEBUG/NDEBUG symbols effectively useless, as the behavior of the program would depend on only ever being in “debug” mode.
shows that they have a fundamental misunderstanding of what asserts are actually used for
This is a microcosm of why I don’t like the style of the OP. Why do we need to make inferences about the programmer’s mental state? Why can’t we just talk about the code rather than leaping to conclusions?
bad programmers tend to use them badly
most programmers don’t really understand
That’s why they
the typically wrong ways bad programmers use asserts
More generally, though, it shows why there’s a difference between 1x and 10x programmers. 1x programmers, like those writing Bitcoin code, make the typical mistake of treating assert() as error checking. The nuance of assert is lost on them.
The technical content of this post is good. I agree with it. But there’s a lot of distracting material around it. Presumably, the goal of writing this post and sharing it with others is to educate others. But the post, while containing educational content, is also insulting the very people it’s trying to educate!
Now, I don’t claim to be an expert in pedagogy, but I can’t recall ever having success teaching folks while simultaneously insulting them. Maybe it’s some sort of “tough love” strategy? I don’t know, but I don’t like it.
Now, if the point of this post was not to educate, but rather, to snub one’s nose, then sure, this post is a home run. But I can’t stand superciliousness either, so posts like this are a lose-lose in my book.
Presumably, the goal of writing this post and sharing it with others is to educate others. But the post, while containing educational content, is also insulting the very people it’s trying to educate!
I agree with this, but this is coming from some Bitcoin source code (I’m still not sure I understand which source tree this is from) and it’s code that deals with financial transactions. The developers have the obligation to their users to ensure that their code is correct for that reason alone. If you can’t trust these devs to use assertions (which help in catching correctness of your program), would you trust them with sending your money through their system? I certainly wouldn’t.
Is the tone of the article harsh? Absolutely. But when you’re writing code that deals with other peoples' money especially, you need to scrutinize every last detail of the code. The quality and practices the authors employ are not exempt from that.
you need to scrutinize every last detail of the code
Please note that I didn’t contest this and very strongly agree with scrutinizing code. I’d be happy to rephrase my point if you like, but I’m not sure where the misunderstanding is.
Sure, I can understand it’s more than an IDE thing. Programs depending on it? .. seems more like a convention (abused?). I am willing to know more about that.
Is there a strict assert adherence code? When was it decided universally that asserts are bad in release code? I believe assert has it’s value even in release.
I don’t disagree with his overall point. Sure, people misuse assert(). There’s lots of bad coders out there who have half-right ideas about things. Everyone has been that person. Ideally we all keep growing. This little gem though.
“More generally, though, it shows why there’s a difference between 1x and 10x programmers. 1x programmers, like those writing Bitcoin code, make the typical mistake of treating assert() as error checking. The nuance of assert is lost on them.”
The tone of this article is very rude and immature. It sounds more like the author is trying to shame the “Bitcoin Unlimited” people for some reason more than he’s really trying to be helpful.
I agree it is rude, but I also feel with author. This is literally code, which wants to handle billions and trillions of dollars worth of Bitcoins. Misusing assert is not something which gives me confidence.
If those were the biggest gripes he could find over the whole code base, then I’d say the code stands up really well actually.
At least two of the issues are ugly but harmless (checking the result of new and the one that checks a big condition and then asserts false). One of them is a non-issue if asserts are turned on in release builds (there’s a compile flag, are they using it?).
The remaining one, using assert to check if data is on disk, is not how I’d use assert, but I can see how it’s justified if the code should really never run when the data isn’t there and there’s no way to recover. I don’t know enough about the codebase and how the system works to judge one way or the other, so I’ll give them the benefit of the doubt.
“These people don’t write code exactly the way I do, therefore they are bad coders,” is not a valid argument, and there’s really not a standard agreed upon rule on how to use assert.
If you are talking about example 2:
The problem there is when NDEBUG is defined, view.Flush() is never called. That’s also an abuse of assert() (calling a function with side effects).
On the contrary, if the code is never compiled with NDEBUG defined then view.Flush() will always get called. For better or worse, lots of applications ship with asserts turned on, even in release builds.
I’m not saying it’s a great use of assert, but it’s also not so bad that the developer deserves to be shamed with an article calling him out as a bad coder, either.
I don’t like the tone of the article. I understand there can be, and there are better practices than using assert.
I don’t like double inversion, this is confusing:
assert(!coins->vout[nPos].IsNull());Assert is disabled with NDEBUG. The Debug vs Release issue the author is talking about seems more like an IDE thing.
Assert isn’t meant for error recovery (if it crashes the program, so be it. It’s better than running the program any further).
Bad Code, Bad Programmers, just seem like random insults thrown about.
The author of the article is pointing out genuine misuse of assert() in the program. Asserts are used explicitly to catch “impossible” program states, thus helping the debugging and development process. They are meant to be redundant. If an assertion fails, it shows that you have a bug in your code. The fact that the original authors of the code are using these as a way to check user input, as well as check if something that has side effects has succeeded, shows that they have a fundamental misunderstanding of what asserts are actually used for.
This is more than an “IDE thing”. Programs depend on these compile-time definitions to include or exclude special debug information - like asserts or compile-time warnings. This source was abusing the use of asserts, which as a side effect, made DEBUG/NDEBUG symbols effectively useless, as the behavior of the program would depend on only ever being in “debug” mode.
This is a microcosm of why I don’t like the style of the OP. Why do we need to make inferences about the programmer’s mental state? Why can’t we just talk about the code rather than leaping to conclusions?
The technical content of this post is good. I agree with it. But there’s a lot of distracting material around it. Presumably, the goal of writing this post and sharing it with others is to educate others. But the post, while containing educational content, is also insulting the very people it’s trying to educate!
Now, I don’t claim to be an expert in pedagogy, but I can’t recall ever having success teaching folks while simultaneously insulting them. Maybe it’s some sort of “tough love” strategy? I don’t know, but I don’t like it.
Now, if the point of this post was not to educate, but rather, to snub one’s nose, then sure, this post is a home run. But I can’t stand superciliousness either, so posts like this are a lose-lose in my book.
I agree with this, but this is coming from some Bitcoin source code (I’m still not sure I understand which source tree this is from) and it’s code that deals with financial transactions. The developers have the obligation to their users to ensure that their code is correct for that reason alone. If you can’t trust these devs to use assertions (which help in catching correctness of your program), would you trust them with sending your money through their system? I certainly wouldn’t.
Is the tone of the article harsh? Absolutely. But when you’re writing code that deals with other peoples' money especially, you need to scrutinize every last detail of the code. The quality and practices the authors employ are not exempt from that.
Please note that I didn’t contest this and very strongly agree with scrutinizing code. I’d be happy to rephrase my point if you like, but I’m not sure where the misunderstanding is.
Sure, I can understand it’s more than an IDE thing. Programs depending on it? .. seems more like a convention (abused?). I am willing to know more about that.
Is there a strict assert adherence code? When was it decided universally that asserts are bad in release code? I believe assert has it’s value even in release.
I don’t disagree with his overall point. Sure, people misuse assert(). There’s lots of bad coders out there who have half-right ideas about things. Everyone has been that person. Ideally we all keep growing. This little gem though.
“More generally, though, it shows why there’s a difference between 1x and 10x programmers. 1x programmers, like those writing Bitcoin code, make the typical mistake of treating assert() as error checking. The nuance of assert is lost on them.”
Aaaahahahahahah oh buddy, get absolutely fucked.
Bitcoin people get very defensive, when challenged. That is what I got from the comments.