[C++-sig] Questions/Comments about new stuff

[David Abrahams]

----- Original Message -----
From: "Dave Hawkes" <daveh at cadlink.com>

> > Also, this function is much too big, dense, and deeply-nested. Comment
> > lines should in general be preceded by a blank line. Many lines are too
> > long. I wouldn't make such a big deal about this one, but I know I
> > mentioned it as early as
> > http://aspn.activestate.com/ASPN/Mail/Message/1230478.
> >
> Now that the API is in place I plan to revise this code as I should be
able
> to make it a little more compact. I'll tidy the style up at the same
time. I
> did revist the code since that discussion but it was difficult to pull
out
> many helper functions that would use the overall code apart from maybe a
few
> thing that would replicate what was being handle in the python API code.
>
> > . What is going on with transfer_attributes()? I don't understand why
it's
> > there, yet. Why didn't you make it accept type_handle arguments instead
of
> > object? That would have made the code far simpler...
> >
> Again this can be reworked using the API code. This code is here so the
> nesting order of classes doesn't impact their order of instantiation in
the
> module initialization.

Well, I'm not even sure it works - witness the iterator crash I'm seeing.
Certainly this seems completely broken:

     current_object =
python::object(handle<>(boost::python::class_<empty_class>(full_name.c_str(
)).object()));

All instances of class_<T> refer to the same object for a given T. What
happens when you have multiple levels of nesting (c1.c2.c3...)?


> > . Variable names like "r" for a type_handle don't say enough about its
> > purpose to help the reader.
> >
> I think that one slipped through when I was changing a few other variable
> names.
>
> > I'm still reformatting and massageing this get_context_object() thing
so I
> > can get a feel for what it's doing...
> >
> It's still only around 30 lines of code excluding the comments and I may
be
> able to par it down further using the API code.

Don't touch anything for the time being. I'm going to fix it up to show you
how it should be done. Adding stuff like this (and especially
init_sub_module) to the codebase, even as a temporary measure to be cleaned
up later, is really unacceptable because it means that I can't maintain it
until it's cleaned up. Note that I don't even think you're able to maintain
code written in this style. If the code were less-dense, you would have not
missed "r", for example. Code has to go into the codebase with a tidy style
to start with, as much as possible.

-Dave