|
Boost : |
From: Peter Dimov (pdimov_at_[hidden])
Date: 2001-08-18 10:18:19
From: "Jens Maurer" <Jens.Maurer_at_[hidden]>
> - I'd like to see the tests merged into one .cpp file so there's
> only one more line in the tables for the regression test results.
The reason for the separate tests is that the library may be usable even
when some of the tests fail. Having a single 'fail' line doesn't provide
enough information.
> bind(&X::f, ref(x), _1)(i); // x.f(i)
> bind(&X::f, &x, _1)(i); // (&x)->f(i)
> bind(&X::f, x, _1)(i);
> bind(&X::f, p, _1)(i);
>
> First, the last two lines should also have a comment indicating the
> semantics. I presume it's "x.f(i)" and "p->f(i)".
The semantics are "(the internal copy of x).f(i)" and "(the internal copy of
p)->f(i)", respectively.
> I'd like to see the BOOST_BIND ugly hack strongly discouraged
Well, everything that is not part of the interface is not guaranteed to work
or persist between versions, but I could try to use some strong words here.
:-)
> and
> the BOOST_BIND macro renamed to something more detesting, e.g.
> BOOST_MSVC_BIND_NAME_CLASH_AVOIDANCE (please find a better name).
This I'm not sure I agree with.
> - The visitor explanation is seriously lacking. What's the
> "internal state" of a bind function object? What requirements
> are imposed upon "v"? Yes, the visitor pattern is supposed
> to be an implementation detail, but on the other hand, it's
> supposed to be used by other libraries, so it's indeed an
> exported feature.
The visitor support is an experimental feature. It will evolve over time
when other libraries start to exploit it; or it may be removed altogether if
no other library needs it. It's not (yet) part of the interface.
> - The get_pointer() approach requires additional
> co-operation from smart pointer classes not required
> when using std::mem_fun_ptr. Therefore, bind's mem_fun
> is not a fully backwards-compatible substitute for
> Boost.Functional's mem_fun_ptr (or std::mem_fun_ptr, for
> that matter).
I don't understand this paragraph. std::mem_fun simply does not work with
smart pointers, AFAIK.
> mem_fun.hpp:
> - Line 35: I believe a forward declaration of
> boost::shared_ptr<> is insufficient here, because the
> get_pointer() implementation calls p.get(), which requires
> a completely-defined type.
get_pointer() is a template; by the time it gets instantiated the shared_ptr
definition will be fully visible. I think. If como says that it's fine then
it's fine. :-)
> - I don't like the _mfi and _bi namespaces. Boost
> uses the "detail" namespace for implementation details. I hope
> that's also stated in the library guidelines somewhere.
It is, but having a single 'detail' namespace is not enough. Name clashes in
detail will become common; boost:: names are documented (most of the time)
but boost::detail:: names are not.
> bind.hpp:
> - Could the implementation use the tuple library?
Yes, it could. Such a dependency has its pros (less work for me) and cons.
For instance, bind.hpp will fail on every platform where tuple.hpp fails.
-- Peter Dimov Multi Media Ltd.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk