Re: [boost] Review: Python/C++ binding library

From: David Abrahams (abrahams_at_[hidden])
Date: 2000-10-10 10:27:52

----- Original Message -----
From: "Gary Powell" <Gary.Powell_at_[hidden]>

> Hi,
>
>  A couple of minor comments.

Thanks for participating!

> File : subclass.cpp
> fn enable_special_methods(..)
>   the second for loop, has is_prefex(..., name + 2)  done for each
iteration
> of the loop, this should be done once outside the loop after the test for
> "!is_special_name(name)" While a good compiler should move this code, why
> not just do it since that's what you mean anyway.
>
>   same comment for enable_named_method

One of us is completely nuts ;->. How can I move the test
(is_prefix(enablers[i].name + 2, name + 2)) outside the loop if it depends
on the loop index, i?

> I may be confused here but it looks like there is a problem in
> Instance::getattr, where it is declared to return a PyObject *, yet can
> return a new BoundFunction();

BoundFunction is derived from PyObject.

> if BoundFunction returns a Ptr

Nobody's calling the BoundFunction object; that's a ctor-initializer you're
looking at.

> and you call
> new on it, is it correctly converted to a PyObject *?, wouldn't it be
better
> to call
>    return BoundFunction(blah blah).release(); ?
> I'm not a big user of auto_ptr<> yet so there may be an idiom I'm missing
> here.

Python eventually expects me to talk to it with raw PyObject* pointers. I
hope that helps explain what's going on here.

> perhaps a comment in the code would clear this up for future maintainers.

I would have thought that this was one of the less obscure areas in this
codebase, but I guess it just goes to show that one can never tell. As soon
as you can explain your confusion to me I will be able to add a comment
aimed at preventing that confusion . I have to understand what you're
thinking first, though.;->

> Namespaces: The one used is "py" and "py::detail" should these become
> "boost" and "boost::detail::py" ? And the code within "detail" moved to a
> subdirectory "detail"?

Naming details and boostification are still pending. Since the library
itself has yet to get a final name (What's your vote?) it would  be
premature to select namespaces, I think.

>  If and when py_c++ becomes part of the standard, won't we have a problem
> with the class names "String" and "Tuple" (I'm assuming that Jaakko
Jarvi's
> Tuple will also make the grade.)

Not if they're properly namespace scoped ;->

> Also on general principles I object to
> naming a class "Object" How about "PyPtrObject" that's what this one is.
> (also "Dict" -> "PyPtrDictionary" ?)

You're forgetting the namespace scope again. These are actually py::Object
and py::Dictionary. Or, if namespace py:: becomes python::, python::Object
and python::Dictionary.

Thanks again for your feedback,

Dave