Re: [Opencxx-users] Makefile.am fixes

[Yann D. <yd...@us...>]

On Tue, Sep 16, 2003 at 09:30:18AM +0800, Grzegorz Jakacki wrote:
> > Yes, but that makes a modified copy of the original code, and will require
> > updating to take advantage of any changes in the original...
> 
> You are right. I would say that this is acceptable on this scale, but that's
> maybe because I am getting old. Let's try again.

On the scale of TranslateCast() I tend to agree (so perhaps I'm getting old
myself as well ;), but I only used this one as an example of what I'd see
useful with most Walker::Translate*() methods.


> I understand that you would like to add BeforeTransalteCast(),
> AfterTranslateCastedExpression(), BeforeTransalteCastedExpression(),
> TranslateCastType() and TranslateCastType() to the public interface of
> Walker. I would like to be careful about it, as changes to interfaces are
> difficult to reverse.
> 
> First reservation is: why do we need FIVE new interface functions? It
> seems to me that at most two should be enough.

Your demonstration below is convincing to me :)

> Too many functions makes interface diffucult to comprehend.

Sure, and furthermore I think the resulting code would be much slower.

> Second: function like 'BeforeTranslateCast()' seems to be a bad candidate for public
> iface function, as it has very complicated postcondition. If you do not
> return some very specific Ptree, the remaining code will fail. This has to
> be documented and understood by the client, so I would like to avoid it if
> possible.

Yes, probably its call would need to be wrapped in a check that would raise
an exception if the postcondition is violated.  And that again would make
the code much slower.

> >From what I can see the problem originates from the fact, that
> 'TranslateCast()' violates Single Responsibility Principle.
[...]
> So maybe we can go on with something like this:
[...]

That looks fine to me.  Probably most other Translate*() methods would
benefit from such a change.

> Perhaps we also can give more meaningful types to arguments of
> TranslateCastedExpression() and TranslateCastedType().
> 
> What do you think?

That may make sense as well.  Looks like it could help to catch silly
mistakes when writing a custom walker.

> > > Looks like we came to similar conclusions.
> > >
> > > The code that I wrote above works, but it violates Liskov Substitution
> > > Principle --- the user cannot be sure about the behaviour of member
> > > function 'TranslateCast()' when it is called via Walker*.
> >
> > Hm - what assumptions of Walker would this code break ?
> 
> See
[...]

I already looked up the lsp to be sure about what you meant.  As I
understand it it refers to derivative classes causing assumptions about the
base class not to be valid any more on the derivative class.  What I fail to
see is which assumptions about the Walker class would the
MyWalker::TranslateCast method as described break.

Regards,
-- 
Yann Dirson    <yd...@al...> |    Why make M$-Bill richer & richer ?
Debian-related: <di...@de...> |   Support Debian GNU/Linux:
Pro:    <yan...@fr...> |  Freedom, Power, Stability, Gratuity
     http://ydirson.free.fr/        | Check <http://www.debian.org/>