Re: some comments about our C++ code...


Subject: Re: some comments about our C++ code...
From: Martin Sevior (msevior@mccubbin.ph.unimelb.edu.au)
Date: Tue Apr 17 2001 - 02:11:03 CDT


On Mon, 16 Apr 2001, [iso-8859-1] Joaquín Cuenca Abela wrote:

HI Joaquin,
           Sorry for taking up your extremely valuable development time
with this. You're right of course.

>
> > Mike Nordell has told me off for doing it but it seemed the best
> > way to implement some things.
>
> Mike always is right.
> Mike always is right.

Yet again I have to agree :-)

> >
> > Specifically if you genuinely need to read and write a variable in a
> > subclass why put an extra layer of complexity between what you need to do
> > and what you're actually doing?
>
> because what you need to do today may be different than what you need to
> do tomorrow. Usually you don't need to read and write a variable, you
> need to get information from a class or you need to mutate a class.

In fact when I think about the trouble I've had with the Lists dialog a
lot of it can be traced to exactly this! I had member variables in XP land
in protected space that were accessed in the platform code. This caused no
end of trouble when I wanted to change code in the dialog! If I'd defined
access via methods this trouble would have been avoided. OK live and
learn.

From now on my XP variables will be private.

>
> > Another example is the fp_PageSize class in PD_Document. It just makes
> > sense to leave this as a publically accessible class. Otherwise one would
> > have to effectively duplicate all the methods in this class within
> > PD_Document to change anything about it.
>
> Ah, this one is good example.
>
> right now we have:
>
> class PD_Document : public AD_Document
> {
> public:
> PD_Document(XAP_App *pApp);
>
> // ... [snip many cool stuff (maybe too many :)]
>
> fp_PageSize m_docPageSize;
>
> // ...
> };
>
> instead of something like:
>
> class PD_Document : public AD_Document
> {
> public:
> PD_Document(XAP_App *pApp);
>
> // ... [snip many cool stuff (maybe too many :)]
>
> const fp_PageSize &getPageSize() { return m_docPageSize; }
> void setPageSize(const fp_PageSize &pPageSize) { m_docPageSize =
> pPageSize; }
>
> private:
> fp_PageSize m_docPageSize;
>
> // ...
> };
>
> so if we want to change the page size of a document, we only have to do:
>
> pDoc->m_docPageSize.whatever();
>
> and we have a pretty natural way to change the page size of a document.
> So far, no problems. Now thinks that some crazy dude does some tests
> and he says: "hey, wait a second!, we're spending 1/2 of our memory in
> instances of m_docPageSize!! and all them have nearly the same content!,
> we should optimize out it!"
>

<snip>

> So, the smart dude changes PD_Document to use a flyweight design pattern
> and keep low memory usage and happy users, and so PD_Document becames
> something like:
>
> class PD_Document : public AD_Document
> {
> public:
> PD_Document(XAP_App *pApp);
>
> // ... [snip many cool stuff (maybe too many :)]
> const fp_PageSize &getPageSize() { UT_ASSERT(m_docPageSize); return
> *m_docPageSize; }
> void setPageSize(const fp_PageSize &pPageSize) { m_docPageSize =
> askWeirdCentralPoolFor(pPageSize); }
>
> fp_PageSize * m_docPageSize;
>
> // ...
> };
>
> ok, so far, we can be proud of ourselves. We found a bug (memory
> consumition too high), we know the solution, and we apply it... except
> that it will not work. Why? Because everybody is using
>
> pDocument->m_docPageSize.whatever();
>
> instead of
>
> pDocument->getPageSize().whatever();
>

OK Here I'm confused. Since the m_docPageSize class is in private space
how can we change it's internal state from outside the PD_Document class?

ie:
pDocument->getPageSize().Set(double w, double h, Unit u)

alters the internal state of the fp_PageSize class. I thought this was
forbidden outside the PD_Document class.

This is the reason I put it in the public space. What am I missing?

Thanks!

Martin



This archive was generated by hypermail 2b25 : Tue Apr 17 2001 - 02:12:02 CDT