Re: [boost] Review: Function library

From: Jesse Jones (jesjones_at_[hidden])
Date: 2001-06-13 07:32:55

I have some quibbles with the design, but I've found this sort of 
callback library awfully useful and the code looks fundamentally 
sound so I think it should be accepted into boost. I tested it with 
CodeWarrior 6.1 on the Mac and the only problems I ran into were some 
niggling little details because I had the warnings cranked up (see 
below).

There are a few design decisions that I'm dubious about:

1) I don't see why policies and mixins should be built into function. 
I think the way to handle this is sort of like the way argument 
binding is handled: clients would call a binder function with the 
function and the pre/post functions and a boost::function would be 
returned that points to an appropriate function object.

2) I'd prefer it if calling a function with an empty target was 
treated as a precondition violation (and handled with an assert). The 
rule of thumb that most people seemed to buy into was that asserts 
should be used where clients can easily preflight and exceptions 
otherwise...

3) In the absence of a bind library it seems silly to force everyone 
to generate their own copies of the bind code in the FAQ. I would 
imagine that nearly everyone who uses boost::function is going to 
need to work with methods so I think their should be a stopgap 
solution inside boost.

I added some new tests to function_test.cpp. At the top I added a 
couple of new functions:

static int do_stuff(int n = 0)	{(void) n; return 0;}
static int write_return_three() { global_int = 3; return 3;}

And in test_zero_args() I added some code:

   // Use default argument
   function<int> d(do_stuff);
   BOOST_TEST(d() == 0);

   // Test compatible return types
   function<long> l(generate_five);
   BOOST_TEST(l() == 5);

   // Ignore return values
   function<void> i(write_return_three);
   global_int = 0;
   i();
   BOOST_TEST(global_int == 3);

   // Test const function's
   const function<int> c(generate_five);
   BOOST_TEST(c() == 5);

Here are the changes I had to make to get the code past the compiler:

1) There are signed/unsigned conversion problems in 
allocator_test.cpp: I changed the allocate and deallocate methods so 
they take a std::size_t instead of an int.

2) In catch_exceptions.hpp I made report_exception() inline (static 
would work just as well).

3) In defarg_test.cpp I made test_two_args() static.

4) In function_test.cpp I made test_zero_args(), test_one_arg(), 
test_two_args(), and test_emptiness() static.

5) In test_main.cpp I made report_error() and report_critical_error() static.

6) I undef'ed nil at the top of function_base.hpp, but as was 
mentioned this isn't going to work in the real world.

   -- Jesse