Boost logo

Boost Users :

Subject: Re: [Boost-users] [signals2][review] Fwd: Signals2 Review
From: Frank Mori Hess (frank.hess_at_[hidden])
Date: 2008-11-03 14:44:18

Hash: SHA1

On Monday 03 November 2008 12:59 pm, Stjepan Rajko wrote:
> * Introduction:
> Is it really important that we know Signals2 was formerly known as
> thread_safe_signals?

No, it's not particularly important. Mostly I put it in because it was still
called thread_safe_signals when I originally requested a review, and wanted
to avoid confusion. I'll remove it if the library gets accepted.

> * Do we need "beginner", "intermediate", and "advanced"?
> I'm not sure why I'm cautioned that the tutorial is not meant to be
> read linearly, especially since the tutorials typically (and
> expectedly) progressed from basic to advanced material. Readers tend
> to pick and choose what they need anyway, and it seemed more visually
> cluttering than anything. I just followed everything linerally, and
> did not seem to have many problems.
> On the other hand, if this is essentially how the documentation has
> been structured for years, it's unlikely there's any need to change
> it.

Yes, that structure (and the vast majority of the tutorial) comes directly
from the original Boost.Signals documentation.

> * Compiler versions and preferred-form table:
> I wonder if, instead of maintaining a list of which compilers
> do support the preferred syntax, it would not be easier to just
> maintain a list of those which do not support it, especially since it
> looks to be that only broken compilers don't.

I actually completely ripped that table out recently, since it really is only
based on compiling the original Boost.Signals library, and I've never updated
it. However, maybe I should bring back the "unsupported compilers" part of
it, since they definitely won't compile signals2 preferred syntax.

A larger question might be whether the "portable syntax" should be deprecated
entirely and removed from the tutorial. I really don't know if any of those
old compilers will even compile signals2 in the "portable syntax".

> * Slot ordering examples:
> In the intermediate example of ordering slots, I wasn't able to change
> the order my signal was displayed after reading the material a few
> times.
> It always came last, no matter what order I added it in. I was able to
> do it, but it was by using the numeric ordering I'd learned in the
> beginner section.
> In my own test program, I tried:
> // ...
> GoodMorning gm;
> HelloWorld hello;
> Newline nl;
> sig.connect(gm); // confused as to why this always comes last
> sig.connect(2, nl);
> sig.connect(1, hello);
> sig.connect(0, gm); // ok, clear why this goes first
> // ...
> ...after playing around a while, I understood what the intermediate
> example was explaining, and it made sense.
> Maybe it's a pitfall of reading the tutorial out of the recommended
> order, but the beginner section didn't really seem to explain to me
> clearly that sig.connect() is actually adding groups and not
> "individual" signals.
> (Or, perhaps I'm still confused about how it works? :>)

Yes, it could be clearer. What is really happening is there are two special
slot groups for ungrouped slots. One has all the ungrouped slots connected
with "at_front" and the other has all the ungrouped slots connected
with "at_back". The ungrouped "at_front" slots are always executed before
any grouped slots, and the ungrouped "at_back" slots are always executed
after all grouped slots. This behavior is taken from the original

> The section where we deal with return values just feels to me like a
> logical extension of the behavior the beginner tutorial has just
> discussed, rather than as a seperate tutorial with significantly more
> advanced behavior-- it seemed natural to want to return values next.

My guess was Doug made those beginner/advanced assignments based on how
commonly someone would want to use a particular feature. I'd guess wanting
to pass arguments from the signal invocation to the slots is far more common
than wanting to receive a return value at the signal invocation from the

> * Blocking tutorial:
> I got a suprise playing with the blocking section when an assertion
> was triggered:
> // ...
> boost::signals2::signal<string (string a, string b)> sig;
> boost::signals2::connection c = sig.connect(&revcatgoat2);
> if(c.connected())
> cout << *sig("a", "b") << endl;
> // ...
> {
> boost::signals2::shared_connection_block block(c);
> if(!c.blocked())
> {
> cerr << "something is wrong\n";
> throw;
> }
> cout << "You shouldn't see this: ";
> cout << *sig("a", "b"); // SUPRISE abort()!
> cout << endl;
> }
> // ...continue, expecting output when block goes out of scope...
> ...The tutorial didn't say anything about that happening. Perhaps I
> was misusing the library..? It worked fine when I checked for
> c.blocked() again before trying to call sig().

Hmm, yes I should add a bit more about the perils of optional_last_value in
the tutorial. What's happening is, the signal is returning an empty
boost::optional because the signal's only connection is blocked when you
invoked it. Then attempting to dereference the empty optional produces the
assert. So what you would want to do is store the signal's returned optional
and check that it is non-empty before dereferencing it.

> * Connection Management example:
> I got a small (but pleasant) suprise when I tried out the connection
> management:
> {
> messageSignal deliver;
> messageReciever *mr = new messageReciever("message reciever
> one");
> deliver.connect(boost::bind(&messageReciever::show, mr, _1));
> {
> messageReciever *mr2 = new messageReciever("message reciever
> two"); deliver.connect(boost::bind(&messageReciever::show, mr2, _1));
> deliver("message");
> }
> deliver("message"); // SUPRISE: no problem
> }
> ...after reading the tutorial section, I had expected the second
> deliver() call to segfault, but instead it worked nicely. I'm not
> really going to complain about this, but I'd expected something
> differently from the tutorial's explanation that it was expected to
> segfault.
> It seems reasonable to expect this behavior, given the explaination
> about shared_ptr<>. Indeed, calling delete explicitly produced mangled
> output. Perhaps extending the example to show an explosion and how
> track() gets around it would be worthwhile.

The tutorial does say in the text that the newsMessageArea object is
destroyed. However, yes it would be clearer if it spelled it out in the code
as well.

> * Automatic Connection Example
> Problem: There seems to be no get() for NewsMessageArea in the
> automatic connection management example, and the signature with the
> application operator appears to probably not be correct. I had to use:
> // Note second parameter to slot_type ctor:
> deliver.connect(messageSignal::slot_type(&messageReciever::show,
> mr2, _1).track(mr2));

The get() is from shared_ptr. Did you notice how in the connection management
snippet, newsMessageArea has been changed from a raw pointer into a
shared_ptr? If you bind a copy of the shared_ptr to the slot (get rid of the
get()) then the shared_ptr will never expire due to the copy living in the

> * Compiler complaint in "Document-View" example:
> button.doOnClick(&printCoordinates);
> ...requires:
> button.doOnClick(&Button::printCoordinates);
> ...the example included in the package did not have this issue.
> (I implemented my own in working through the tutorial.)

In the tutorial, printCoordinates is a free function. You must have
implemented it as a member of the Button class?

> * Comment on "Passing Slots" example:
> In the "passing slots" example, Button::f() never appears to get
> bound. A complete example would be nice.

Hmm, yes I think

void f(Button& button)

could just be replaced with something like

Button button;

since f() isn't used for anything.

Version: GnuPG v1.4.6 (GNU/Linux)


Boost-users list run by williamkempf at, kalb at, bjorn.karlsson at, gregod at, wekempf at