-----BEGIN PGP SIGNED MESSAGE----- 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 Boost.Signals.
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 slots.
* Blocking tutorial:
I got a suprise playing with the blocking section when an assertion was triggered: // ... boost::signals2::signal
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 slot.
* 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) { button.doOnClick(&printCoordinates); } could just be replaced with something like Button button; //... button.doOnClick(&printCoordinates); since f() isn't used for anything. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFJD1SS5vihyNWuA4URAsiJAJwPEG4tZaRRNuoeoUlthsOUU873CgCgzpfB cBDIRt/gz7crhSe+k9VkNmU= =HxYi -----END PGP SIGNATURE-----