subclassing fp_Run (was) Re: Backend changes ready for commit - please test it!


Subject: subclassing fp_Run (was) Re: Backend changes ready for commit - please test it!
From: Martin Sevior (msevior@mccubbin.ph.unimelb.edu.au)
Date: Wed Feb 21 2001 - 18:34:09 CST


C++ gods please read!

On Wed, 21 Feb 2001, WJCarpenter wrote:

> jesper> My point, and it still stands after your little math job, is
> jesper> that doing _anything_ for 99.9999% of the lines to handle a
> jesper> little quirk needed for the odd 3-line footer in every 10th
> jesper> document is a hack whichever way you try to defend it :)
>
> Even if this performance point is moot (and I believe it is because it
> all washes out in the vtable overhead anyhow), we're using C++ classes
> for abstraction and encapsulation of behavior, and so subclassing
> "something just like that but a little bit different" is the way to go
> to favor good maintenance.

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.

What I thought you were suggesting is a sub class of fl_BlockLayout
which would construct sub classes of fp_Run and fp_Line

ie HdrFtrSectionLayout-> fl_HdrFtrBlockLayout -> fp_HdrFtrRun
                                              -> fp_HdrFtrLine

Upon further investigation of the code I realize that this isn't
sufficient. Runs are contructed all over the place in the formatter...

OK for the sake of making my point lets confine ourselves to
fl_BlockLayout

Doing this means rewriting all the methods in fl_BlockLayout which
contruct the runs and lines with identical code except for

new fp_HdrFtrRun <--> new fp_Run fp_ColumnBreakRun fp_FieldRun
fp_TextRun...

so instead of

fp_TextRun *pRun = new fp_TextRun* pNewRun = new fp_TextRun(this, pG,
m_pFirstRun->getBlockOffset(), 0);

we need to write

fp_HdrFtrTextRun *pRun = new fp_HdrFtrTextRun*
fp_HdrFtrTextRun(this, pG,m_pFirstRun->getBlockOffset(), 0);

in every method with a "new fp_run" sub class in it. There are 41
"new fp_Run"'s in fl_BlockLayout! Clearly that much
duplicated code with a single difference is a maintence nightmare.

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

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....

Not only in fl_BlockLayout but in all classes where runs are constructed.

I submit that it is much more maintainable to pass m_bisHdrFtr into the
constructor of fp_Line and fp_Run and deal with it in draw(),clearscreen()
and underlining methods.

so now I just have to change:

fp_TextRun *pRun = new fp_TextRun* pNewRun = new
fp_TextRun(this, pG,m_pFirstRun->getBlockOffset(), 0);

to

fp_TextRun *pRun = new fp_TextRun* pNewRun = new
fp_TextRun(this, pG,m_bisHdrFtr, m_pFirstRun->getBlockOffset(), 0);

Furthermore if I do this in the definition of the fp_Run constructor then
the compiler will complain and tell me if I've missed something somewhere.

Now I think about it I don't think we should call the variable
m_bisHdrFtr. I think it should be called m_bisVirtual because this means
this block/line/run is never drawn on the screen. This could be useful for
things other just header/Footers. It could be used for other objects that
are duplicated throughout the document. Hmm maybe for even really
sophisticated fields....

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.

Cheers

Martin
(Still a C++ newbie)



This archive was generated by hypermail 2b25 : Wed Feb 21 2001 - 18:34:27 CST