This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [setup PATCH] SetDlgItemFont


> On Sat, 2003-08-02 at 09:12, Max Bowsher wrote:
>
> > Now, I don't think moving 2 lines of code and 4 lines of comments into a
> > seperate function makes this clearer - rather, it obfuscates what is
> > happening here.
> >
> > In case you are not convinced, here is the alternate patch, to avoid another
> > round-trip of emails:
> > I'm not particularly fond of the name "SetFontPolicy". Better names
> > welcomed.
>
> call it void PropertyPage::setTitleFont().
>
> And I'm not convinced - this is appropriate to be a separate function.
>
> Approved - as a separate function, called setTitleFont.
>
> Cheers,
> Rob
>

I have to agree with Rob here.  What really should be done down the road[1] is
to have another class derived from PropertyPage, say SetupWizardPage, which is
then the base class for the various pages.  The intended-to-be-rather-generic
PropertyPage really has no business setting fonts.  Keeping this functionality a
separate member will help prevent it from becoming entangled in the core
PropertyPage logic, and make it easier to move to a new class in the
aforementioned future.

"And I even like the color" (Sultan of... Whereever... in "Indiana Jones and the
Last Crusade").  "setTitleFont" is about as good a name as I can think of for
setting title fonts.  (I'll fix the capitalization later ;-)).

BTW, if I haven't said so already (memory is the second thing to go) I
***REALLY*** appreciate your work Max in shepherding my work though the
often-agonizing approval process.

[1] Key words: "down the road", in accordance with the "Bird In The Hand Is
Worth Two In The Bush Pattern" from my upcoming book, "Don't Start Nothin',
Won't Be Nothin': Getting Software From Here To There In Finite Time".

--
Gary R. Van Sickle
Brewer.  Patriot.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]