Result should use safe bool idiom


#1

Looking at the Result class, I think it suffers from the problems found in this article:
http://www.artima.com/cppsource/safebool.html

(To be honest, I didn’t look THAT hard, but I see that Result overloads operator bool)


#2

Hmm, that looks like overkill to me. I did give the class some private int and void* cast operators, to catch most of the things that are likely to go wrong, I can’t think of any circumstances where people are likely to run into any problems with the class.


#3

The author claims this about operator bool():

I tried a few of the examples and the compile errors are definitely confusing. As for the “perfectly valid conversion and overloads” I can’t think of any. One thing I did notice though, declaring a Result on the stack is bulky:

Result result1 (Result::ok()); // This works

Result result2; // COMPILE ERROR

#4

Yeah, that’s deliberate - I’d rather force you to be explicit about whether it’s a success or fail, because neither of those is an obvious choice for a default state.


#5

Vinn …you’re right, but all the examples given in the quoted article look like it’s deliberately wrong code. I mean that, ok it could be missused, but do you really think any serious C++ programmer could do this by accident ??


#6

You’d be amazed at what comes up when you start diving in to very heavy template meta-programming…


#7

You’d be amazed at what comes up when you start diving in to very heavy template meta-programming…[/quote]

Yeahn you’re absolutely right, that’s why I stopped diving in very heavy template meta-programming …


#8

:mrgreen:


#9

I agree that his samples of code that will give wrong results are perfectly plausible if you’re using complex templates, particularly other peoples.

Unfortunately, it’s often the case that your C++ code uses other libraries that do use a lot of template metaprogramming - for example, boost, but there are many others. I have a personal library for Juce that doesn’t even really do metaprogramming - it’s just there so that I can easily create callbacks and run things asynchronously, like this:

[code]
class Foo {
// …
void someFunction(int x, bool y);
};

// …

thread::callAsync(this, &Foo:someFunction, 27, false); [/code]

I’ll bet that I could easily run into the traps in this article that way!

I do think juce::Result would be better if it used the Safe Boolean idiom. But let me be perfectly frank - this problem will (mainly? entirely?) result in weird, hard-to-diagnose C++ compilation errors in some cases, and unfortunately, these are common enough in a C++ application that you simply get good at these or you sink and drown.

Still, I’d vote for using Safe Bool on Result, if it were me…


#10

SEXY!!! This looks exactly like CallQueue and ThreadWithCallQueue !!

I’ve encapsulated SafeBool in an easy to use template, and also put together a very robust Error object that uses it.


#11

The difference between your coolnesses callAsync and its cousins makeCallback, makeThread and runInNewThread is that I can handle functions, functors and method calls, and have “any” number of arguments (well, really only 0 to 4 arguments, but it’d be trivial to extend it).

A significantly earlier version of this code is here. With the slightest provocation :smiley: I’d be willing to put up the newest code…


#12

Yeah I have all that too, but the real coolness of CallQueue is that Listeners is built on top of it, and both use a wait-free custom memory allocator.


#13

Very deluxe for digital audio!


#14

Hmm…I just looked over all of your thread and callback code and it seems that we are addressing two completely different use cases (correct me if I’m wrong but you’re creating a new thread to handle each callback).

The purpose of the objects in my library is to implement concurrent synchronization without using CriticalSection objects to protect mutable data, not to perform lengthy asynchronous operations (although I do have a facility for that, see InterruptibleThread’s idle function).

To this end I provide the GuiCallQueue for synchronizing shared data in the message thread, a ManualCallQueue for synchronizing shared data in the audioDeviceIOCallback (or any other system or library-provided thread), and a ThreadWithCallQueue for synchronizing your own thread that performs tasks in the background (for example, a separate thread which scans through a library of audio files and extracts meta-data).

I don’t mean to sound boastful…it’s just that I’m well, I guess you can say “highly competitive” lol :smiley: :smiley: :smiley:


#15

Tom thanks, upon consideration of your code and my targeted use-case I should document that functions placed into a CallQueue need to follow the same restrictions as code that executes in the audioDeviceIOCallback.

Although this should have been obvious to me after writing the example for ManualCallQueue


#16

Moving this discussion to http://www.rawmaterialsoftware.com/viewtopic.php?f=2&t=8802


#17

Oh, I don’t necessarily create new threads - actually, in my specific code today I have a very specific, small number of threads.

What’s up there is months old and was only a part of what I had even at the time.

No, this is pure C++ code to uniformly wrap things for callbacks. The threads/callasync part is an off-shoot from that!


#18

Ah, I see what you mean. I had something similar, to replace boost::bind. But now, all development environments supported by JUCE have some form of bind() so I switched to using that. Older versions of MSVC and others have it in std::tr1::bind(), while c++11 puts it in std::bind(). In my library I do some environment preprocessor checks and includes, then lift both bind() and a few of the placeholders (e.g. _1) from the appropriate namespace so they can be accessed with a distinct identifier.