Pages:
Author

Topic: Bitcoin source code is a giant mess - page 5. (Read 10776 times)

legendary
Activity: 2053
Merit: 1356
aka tonikt
June 04, 2013, 09:35:17 AM
#17
Coding in C and never using goto, is like driving a car and never using its last gear. Safe for poor drivers... Wink

Bitcoin is written in C++ (which is not even close to C as any C++ programmer knows).
Really?  Well, then I was wrong all my life, believing that C++ was backward compatible with C... Smiley
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 09:33:45 AM
#16
Coding in C and never using goto, is like driving a car and never using its last gear. Safe for poor drivers... Wink

Bitcoin is written in C++ (which is not even close to C as any C++ programmer knows).
legendary
Activity: 2053
Merit: 1356
aka tonikt
June 04, 2013, 09:28:10 AM
#15
Coding in C and never using goto, is like driving a car and never using its last gear. Safe for poor drivers... Wink
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 09:15:31 AM
#14
2. Excessive use of C++ subclasses and operators makes it hard to read code. You see a << b, but it actually goes to a very specific place which is not entirely obvious where.

Streaming operators << and >> have always been the standard way of doing I/O in C++ so are you saying these operators are being used for something *other than streaming*?
full member
Activity: 200
Merit: 104
Software design and user experience.
June 04, 2013, 09:12:19 AM
#13
"goto err" is not a big deal in my view.

Bitcoin-QT is a mess for entirely different reasons.

1. Endianness is not well documented. libbitcoin flips some bytes where BitcoinQT does not, but also does not document why they are doing that.

2. Excessive use of C++ subclasses and operators makes it hard to read code. You see a << b, but it actually goes to a very specific place which is not entirely obvious where.

3. Some utility class names look like variable names.

4. Some funny design patterns are not documented even with a single line of text. E.g. CBigNum is smartly done (BN_CTX and BIGNUM are wrapped in classes differently), but you have to be really proficient in C++ or spend half a day deciphering the code to understand why it is done that way. For someone coming from C/Objective-C it's not easy.

I'm working on Mac wallet app and reimplement many things that BitcoinQT does in C and ObjC and learn how that whole thing works. It is smart, but it's messy.
legendary
Activity: 1064
Merit: 1001
June 04, 2013, 08:52:20 AM
#12
You're a OpenCoin employee / currently hired by OpenCoin?

No
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 08:48:38 AM
#11
It's doable but it needs to be a design criteria from the beginning.

Indeed - I did it with an Object Database I created (the error string buffers are pre-allocated before the ODB instance is even created) - it is a little tricky though as you end up using "non-standard" exceptions (although you can make it enough like a std::exception to be useful).
hero member
Activity: 518
Merit: 500
June 04, 2013, 08:48:25 AM
#10
You're a OpenCoin employee / currently hired by OpenCoin?
legendary
Activity: 1064
Merit: 1001
June 04, 2013, 08:47:04 AM
#9
...I doubt there is much code out there at all that can really handle out of memory conditions well (for a start you can't throw a std::runtime_error as that needs a std::string to be allocated).

It's doable but it needs to be a design criteria from the beginning.
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 08:46:09 AM
#8
I seriously doubt that Bitcoin is written robustly throughout to handle out of memory conditions.

Actually I doubt there is much C++ code out there at all (apart from perhaps OS level code) that can really handle out of memory conditions well (for a start you can't throw a std::runtime_error as that needs a std::string to be allocated).
legendary
Activity: 1064
Merit: 1001
June 04, 2013, 08:44:28 AM
#7
Well in my code you would have seen something like this:
Code:
   if (!pkey || !group || !pub_key || !priv_key) 
      throw runtime_error( "no keys or group found" );

Yeah I agree, I don't see how it is useful to return an error in this case. You want the program to halt. I seriously doubt that Bitcoin is written robustly throughout to handle out of memory conditions.
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 08:43:10 AM
#6
Yup - just checked the source link - error handling.

Well in my code you would have seen something like this:

Code:
   if (!pkey || !group || !pub_key || !priv_key) 
      throw runtime_error( "no keys or group found" );

BTW - I hate the term RAII (although it stuck) - I argued years ago on comp.lang.c++.moderated to call the idiom "scoped objects". Smiley
legendary
Activity: 1064
Merit: 1001
June 04, 2013, 08:42:22 AM
#5
Can we have some context on how it was used?

Yep I updated the original post to include a link.

...These situations include: multi-level breaks, resource allocation/deallocation, error handling in C language, computed goto in Perl language.

In this case its error handling in the C language (although Bitcoin is written in C++).

Everybody is afraid of breaking it since it appears to work, and that is reasonable to an extent, but on the other hand this code base is utterly unsuitable for serious work.

Yep. A year ago I came in here and I wanted to work on Bitcoin. Clean it up a bit, reduce external dependencies, etc... but I was pooh-poohed. Now I'm doing the same work in Ripple, and it is welcomed. For Bitcoin code to improve first the people working on it need to acknowledge that there's a growing problem and then commit to addressing it. Even if it is just a little bit at a time, or for example making sure that new code is clean (adding RAII utilities where needed, for example), it can help to reverse the spaghettification.
legendary
Activity: 2058
Merit: 1452
June 04, 2013, 08:38:10 AM
#4
Can we have some context on how it was used?

Also:
Quote
While overall usage of gotos has been declining, there are still situations in some languages where a goto provides the shortest and most straightforward way to express program's logic (while it's possible to express the same logic without gotos, the equivalent code will be longer and often more difficult to understand).

These situations include: multi-level breaks, resource allocation/deallocation, error handling in C language, computed goto in Perl language.[15][16]
hero member
Activity: 714
Merit: 500
Martijn Meijering
June 04, 2013, 08:36:00 AM
#3
Gavin has said that the genius doesn't lie in the code base, but in the algorithms and that the code is crap. I agree with that assessment. There's a somewhat refactored version available that can also be used for alt coins, but I don't have a link handy right now. I've been thinking about refactoring the code myself. Everybody is afraid of breaking it since it appears to work, and that is reasonable to an extent, but on the other hand this code base is utterly unsuitable for serious work.
legendary
Activity: 1890
Merit: 1086
Ian Knowles - CIYAM Lead Developer
June 04, 2013, 08:33:09 AM
#2
I think this would very much depend upon the specific context - although I haven't coded a "goto" myself since the 1980's I did work on some C code with huge *case* statements in the 90's where it can actually make some sense (in general it is not a good idea with C++ due to the problems of exception handling).

Typically the only reason to use such a construct would be to "skip" to the *end* of a large block of code (and of course you could always use *break* inside a loop to do the same thing).

In no way is a *goto* more "efficient for the compiler" though (nor is it very good for human eyes as no-one would *expect* to even *see* a goto these days) so in regards to your quote I'd have to agree it doesn't inspire a lot of confidence.
legendary
Activity: 1064
Merit: 1001
June 04, 2013, 08:30:20 AM
#1
Presented without comment:



link (github.com)
Pages:
Jump to: