Re: [pygccxml-development] Function Transformation - comments

[Roman Y. <rom...@gm...>]

On 9/4/06, Matthias Baas <ba...@ir...> wrote:
> > 1. code_creators.mem_fun_v_t and mem_fun_v_wrapper_t
> >     I think that for a time being it is better to derive another class
> > from calldef_t and
> >     calldef_wrapper_t. There are few reasons I think why it should be
> > done this way:
> >     1. safety - we can play as much as we want, without breaking
> > existing code
> >     2. you added a lot of code to the classes, I think we will better
> > understand the
> >         whole feature if it will be separated from the existing code.
> >
> >    So I did it for you. I did not add the code you send.
>
> I didn't do an update yet because I think this will lead to quite a few
> conflicts now (I've noticed that you modified calldef.py as well).
> So how do you want me to proceed? Stick all my stuff into the
> mem_fun_v_transformed_* classes?
> Are you planning on keeping those classes (transformed and
> non-transformed versions) separate permanently or is this only for as
> long as this is work in progress?

I don't know. Right now it seems that I will keep them forever, but
tomorrow we could
be smarter and than we will make some refactoring.

> By the way, I think the "standard" way to approach this problem would
> have been to create a branch
> (http://svnbook.red-bean.com/nightly/en/svn.branchmerge.html). But the
> repository layout doesn't seem to be prepared for this option.

You are right, it is not prepared. We will see how it works and if
needed will create a
branch.

> > 2. I don't understand the role of substitution_manager_t, but I'd like
> > to comment
> >    on it's implementation. Why don't you use dictionary and standard
> > Python string
> >    substitution mechanism?
> >
> >    "%(return_)s" % dict( return_="123" )
>
> I didn't use this because there are some variables that may contain an
> entire block of source code (i.e. multiple lines) and each line should
> be indented with the same depth as the variable in the template string.
> Using the Python formatting functionality only the first line would have
> been indented.

If this is the only reason, than I have a solution to it - start every
variable from new
line and format its code before you pass it to the dict. After this
remove duplicate lines.

> > 3. I don't understand subst_t hierarchy.  For example code_manager_t
> > derives from
> >     subst_t.  subst_t class perform text substitutions, while
> > code_manager_t manages
> >     pieces of source code for a C++ function. It seems that subst_t
> > class should be an
> >     implementation details of code_manager_t class.
>
> You mean I should have used delegation instead of derivation?
> Well, I was considering this for a moment and decided for derivation
> because...
>
> - ...the code manager is also supposed to do substitutions but with a
> specialized set of variables. So the "is a" relationship is given.

I don't argue that they both works on strings. I argue that they have different
responcibilities and thus have to be separated.

> - ...I would have to duplicate the definition of substitute() when using
> delegation.

Implementation details.

> - ...the user still has the option to attach variables right to the code
> manager and use those for substitutions (it's a rather special case but
> who knows when it might be handy).

I am a fun of "agile software development" methodology. In our case it
means next:
we don't have use case, we don't write a code.

> > 4. function_transformer_t completely operates on string level. It does
> > not even contain
> >     reference to declaration it works on.
>
> Yes?
> (at least in my cases, I never needed to know the declaration)

And the main problem with it is that user can not understand what
transformation
does. I expected function_transformation to expose "transformed_function_type".
Thus it could be very clear what is done.

> > I expected to see something completely different. I thought about
> > defining hierarchy of
> > "transformation" classes, then the role of function_transformer_t could
> > be an
> > aggregation of all transformations that user want to do for the function
> > to convert it
> > to another function ( 1:1 ).
>
> The function transformers are individual and independent pieces of code.
> The aggregation is done by the substitution manager.

I could be wrong, but it seems that you think about function_transformer_t as
argument_policy. I will try to write a document that explains my
vision of the feature.
(Not a code, I am not going to write single line of code)

Also another problem is that you mix "configuration" classes with
classes that create
a code.

> > Obviously, user will have to write some
> > "glue code" to help
> > all these transformations to work together and we should find the right
> > place to it.
>
> What "glue code" do you mean? As mentioned above, it's the substitution
> manager that "glues" together the individual pieces.

I am not sure. I will try to explain it in the document.

> > Also, what I did not see is responsibilities and contracts. I mean you
> > wrote documentation
> > but it is not clear from the class interfaces.
>
> What parts of the interfaces didn't you understand?
>
> > 1. Why do you start to count arguments from 1 and not from zero?
>
> An index of 0 occasionally refers to the return value.

Problematic: 0 argument in Python is "self", in C++ it could be
"this". This is something
you have to explain and people will have to become accustomed.

> > 2. Why Output is not according to coding convention?
>
> Well, I left this one for the time being as this is actually a class
> that belongs into the "high level" realm and the conclusion in the wiki
> on this topic was to use the Python style guide.

I don't remember that. This is not what is written here:
https://realityforge.vrsource.org/view/PyppApi/DslDiscussion

-- 
Roman Yakovenko
C++ Python language binding
http://www.language-binding.net/