On 8/12/07, Jake Voytko <jakevoytko@gmail.com> wrote:
Stjepan,
I finally finished this week's task for GSoC, so here is my review of your headers:
Yippee!!! Thanks for doing this :-)
***1. Documentation*** 1.0: What I like: - You provide excellent motivating examples - You have good arguments for why one should use the library - Your concepts are well thought out - You have extensive code examples that clearly have taken you a lot of time and thought to produce - All of the code examples provide a good depth of what the library is capable of doing
Great
1.1: Nit Picks - Table 1. is hard to read on my monitor, and determine what, if any, hierarchy exists
Agreed - it's currently a docbook table and as such does little to visually convey the relationships - now that it is starting to stabilize I will put some effort into a better visualization.
- The section titled "Namespace Use" should be in a section called "How to use this documentation"
You are right - most of the stuff in there has more to do about the docs than about the organization. Actually, adding a "How to use this documentation" section would probably do wonders for the usability of the docs. Thanks for the suggestion.
- In the section named "Dataflow.Signals - based on Boost.Signals", you should indicate that the example encountered previously in the documentation is Dataflow.Signals, or provide another example
OK.
- The section labelled "Creating your own signal receiver (slot)" should continue using the "fused vs unfused" table paradigm
I would agree - although I might rethink the whole fused vs. unfused example convention. In so many cases they are identical except for fused vs. unfused, so I'm not sure whether having two almost identical examples is useful or confusing. When it comes to user code though, developing a fused vs. unfused component is usually much different, and in this case a side by side comparison would definitely be useful, as is the case with the doc section you mention.
- On page "Disconnecting", 0u is not a number.
I meant 0 unsigned - I think I was getting a warning if I just put down a zero because it was being compared with size_t or something like that. Is there a problem I am not seeing?
- You should provide examples of compiling/linking your program for those who do not exclusively use bjam.
[cringe]... OK :-)
- There should be a table for all of the components that acts as a "use me when..". When I was trying to write my example, I had only a vague idea of when I should use any of the components, and wasn't sure if I was doing something wrong. I used primarily filter<> and counter<>, as my storage needs were more complicated than I could discern the storage<> class being helpful (I may be wrong.. I only had a few hours to try to do this).
Ooh... I really like this idea.
***2. The library:*** 2.1: What I Like: - The "blueprint" layer sounds fantastic.
Cool! Now I just have to implement it :-) I think this one has been postponed the longest (definitely a post-gsoc undertaking) since I think it might benefit from the reflection and user-friendly graphs libs.
- It is very generic, allowing for all kinds of possibilities, but provides enough specific examples that one can see how it would be useful
I am really happy that it gives that impression.
- I like the networking examples using Boost.Asio
Oh yeah... still gotta implement that async receiver :-)
2.2 Questions: - What is useful about a "chain"? Is it that operator() is called n times on the object it receives, where n is defined by the template parameter?
A chain of, say, 10 components will actually create 10 instances of the component and chain them together (the output of the first connected the input of the 2nd, etc.). The input into the chain goes to the input of the first component, and the output of the chain comes out of the last component. With simple components, the effect is pretty much like invoking one component n times, but the difference would come in when each component could have an independent state that would influence the processing. This, of course, should all be added to the docs :-)
2.3 What could use improvement: - Is there any way you can include something that takes a function pointer of an existing function and makes a function object from it?
// Not sure off the top of my head this would work, but something along these // lines could be nice #include <cmath>
filter<double (double), unfused> my_filter = make_filter_unfused(&sin);
The "function" component does something like this. It can take anything that can be stored in a Boost.Function, and convert it into a filter so that the input to the filter is sent as an argument, and whatever the function returns is sent out as the output. I just looked at the docs for function and realized that they aren't clear at all. No wonder it went unnoticed :-)
***3.0 Sample program*** I tried to make a sample program, but ran into problems. gcc threw errors when I attempted to connect() two components, saying "error: invalid use of undefined type 'struct boost::dataflow::extension::signals::get_slot<(void ()(const std::string&), ....>". I'm not going to ask you to debug it, and I got a pretty good feel for everything by writing the code.
Actually, I would really love to look at your code, because it would give me a glimpse of how you wanted to use the library (it would be especially useful to see it since it didn't let you use it that way). It would help me make the lib more user friendly and/or find some bugs that may have been preventing you from using the lib in what should have been a legal way.
Overall, I felt that the experience was a little cumbersome, but that for complicated networks the benefits would far outweigh the extra time it takes to define individual components. I merely attempted to do some string processing, which could have easily been dealt with in C++ through more conventional means, but I can see how the plug-and-chug methodology makes algorithm design much easier, were one to have many pre-connected components.
Mmm... "plug-and-chug"... I like the sound of that :-) Thank you so very much for taking the time to check out the lib and write up your feedback! It is very insightful and helpful.
Good work so far, Jake
Ditto! Stjepan