Subject: Re: subclassing fp_Run (was) Re: Backend changes ready for commit -please test it!
From: Mike Nordell (tamlin@algonet.se)
Date: Wed Feb 21 2001 - 20:21:39 CST
Martin Sevior wrote:
> OK I've thought about this a bit more and unless I misunderstand something
> about C++ subclassing I think your solution is a much bigger maintenance
> issue.
Perhaps the design is flawed? Perhaps some work is done in the wrong class?
What about not giving runs even more members, but letting the HdrFtrLayout
not forward the draw() calls to the runs it is responsible for? Am I
understanding the issue here, a run should often not be drawn when layed out
by a HdrFtrLayout?
This would void the need for new run and line types.
But this also raises a more serious issue: Should a run really know anything
about the world which it lives in?
> so instead of
>
> fp_TextRun *pRun = new fp_TextRun* pNewRun = new fp_TextRun(this, pG,
> m_pFirstRun->getBlockOffset(), 0);
What's this? Parts of it resemble C++, but the complete expression is just
jibberish. If you remove "fp_TextRun *pRun = new " it makes sense, but I
fail to see the purpose of this. Copy'n'paste error?
> Ok well suppose we don't sub class all of fl_BlockLayout but instead
> choose to construct either a regular run/line or a hdrftr one based on a
> new member variable m_bisHdrFtr
Please, use only *one* variable naming convention at a time. We are to use
HN (Hungarian Notation) so let's not mix in a bit of Java or *nix naming
conventions in this. To be AbiWord correct this would have been
"m_bIsHdrFtr". To use Java or *nix naming conventions with an added C++
member whart it would have been "m_isHdrFtr". But to mix these resulted in
"bis" which doesnt satisfy any of these criteria (V.42bis anyone?). And
again, why should a run have to know if it is part of a header or footer
layout?
> then for every "new", we have to code
>
> if(!m_bisHdrFtr)
> fp_TextRun *pRun = new fp_TextRun* fp_TextRun(this, pG,
> m_pFirstRun->getBlockOffset(), 0);
> else
> fp_HdrFtrTextRun *pRun = new fp_HdrFtrTextRun* pNewRun = new
> fp_HdrFtrTextRun(this, pG,m_pFirstRun->getBlockOffset(), 0);
>
> Whoops! It is worse than this because the scope of pRun is just in the
> "if". I can't actually think how you would elegantly code this....
I don't think we really care _what_ type of run we have created once we have
created it, why the following possibly would suffice:
fp_Run* pRun = m_bIsHdrFtr ? new fp_HdrFtrTextRun(...) : new
fp_TextRun(...);
or even better, create a factory for runs:
fp_Run* = CreateTextRun(m_bIsHdrFtr);
> Now I think about it I don't think we should call the variable
> m_bisHdrFtr.
Agreed.
> I think it should be called m_bisVirtual because this means
> this block/line/run is never drawn on the screen.
<sigh>
The draw method of runs are to be called by, whom? Any of the layout
classes? This is at least in my mind the only reasonable implementation and
logic. Then, it's as simple as falling off a log for the HdrFtrLayout class
to _not_ call the draw() method of the runs.
> OK writing this email has taken a lot of time. I think I'm right about C++
> constructors and sub-classes. If I'm not I' hope a C++ expert will set me
> straight.
You are right. Though you still think too much in data, not interfaces.
Don't worry, this is quite common having a C background (been there, done
that, the transition took some time).
For some amusing, and yet quite educating, reading about C++ you (all) might
want to have a look at the on-line C++ Experts Forum in C/C++ Users Journal
(www.cuj.com). Especially the stories written by Jim Hyslop and Herb Sutter.
/Mike
This archive was generated by hypermail 2b25 : Wed Feb 21 2001 - 20:20:24 CST