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: [SetupXP] The two styles for handling activation refusal


I'm still letting you guys fight this out, but I'm going to snipe from the
sidelines ;-):

[snip]
> > I do not see "bool OnActivate()" as being confusing, nor as less intuitive
> > that firing 2 event handlers consecutively.
>
> There is only one handler. I'm glad that it wouldn't confuse you though
> :}.
>

It will confuse anybody who is used to how Windows does things.  And semantics
aside, OnAcceptActivate() or whatever you want to rename it is handling a
message from the base class, ergo a message handler.

> > It is true that every instance
> > of OnActivate has to start returning a value, but I don't see this as
> > incorrect.
>
> I do.
>

It's not incorrect, you just don't like it.

> > Quoting Robert:
> > > Changing OnActivates meaning would conflate things in the method, and
> > > make the code harder to read - "what does false mean - that Activate
> > > failed?".
> >
> > I think activating, or deciding not to activate, are tightly bound
> > activities, which *should* be handled together. And yes, false does mean
> > that Activate failed - because the page's internal logic chose to make it
> > happen.
>
> No, it doesn't.
> - failing indicates a /problem/.

Throwing an exception would indicate a problem actually, if you want to get all
by-the-book about it.

> - not being activated indicates the page shouldn't be shown.
>
> mixing the two up is exactly the sort of conflation I was referring to
> as occuring when the two aren't separated. It's ALREADY confused you, as
> your statement above indicates: the Antivirus refusal is /not/ a
> failure.
>
> > Quoting Robert's Pro and Con points:
> > Pro1: * Factor out conflated functionality
> > Pro2: * Step us toward some form of MVC pattern
> > Pro3: * Improve readability
> >
> > Con1: * Add additional logic to an extant method.
> > Con2: * Conflate GUI logic with processing logic.
> > Con3: * Make setup harder to read.
> >
> > Addressing point-by-point:
> >
> > Pro1, Pro3, Con3 : IMO, OnActivate is the logical and intuitive place to put
> > this functionality.
>
> Again, it's not. It's the place a lot of programmers would put it - but
> they would rue it in 6 or 12 months.

...while we argue about it for 6 to 12 months.

> From an OOP point of view, the
> evolution might go something like:
> bool PageFoo:OnActivate() {
>   if (some test)
>     return false;
>   ...
>   return true;
> }
>

Nope.  Common case:

bool Whatever::OnActivate()
{
	...
	return true;
}

> "damn, why do I have to hard code this into every derived page.

A return code.  Please.

> It's
> getting tiresome, and we've had juniors adding pages by copying others
> and leaving inappropriate tests in place.

Which could just as well happen in your preferred scheme Rob, come on.

>We also had some guy throw a
> fatal exception from the parent when activation 'failed' - (s)he didn't
> understand that false isn't a failure. That wasn't that bad though, it
> was when the page they had been working on DID fail, but only returned
> false and left the entire app unstable that we really had to scramble to
> fix things up.
>
> I really need to make it /clear/ that all we are doing is allow the page
> to decide itself whether it can be activated..."
>

"Does the OnAcceptActivate() call happen before or after the call to
OnActivate()?  The normal Windows way is to return a code from OnActivate(), so
it must be after, but..."

We can come up with doomsday scenarios until... well, doomsday... and still not
have a bigger chooser.

>
> > Pro2, Con1: IMO, the extra logic truly belongs here, and any other placement
> > contravenes Pro3, Con3.
>
> Sorry, I don't buy this. It doesn't have anything to do with an MVC
> pattern. Altering the signature DOES add additional logic to an extand
> method, which IS CON1. Saying that the logic /truely/ belongs there
> doesn't alter it invoking con1.
>

I'm getting dizzy from all these ProX's and ConY's.

> > Con2: As a piece of GUI logic, the page class is an appropriate location.
> > If, in the distant future, we gain an interactive text UI, then there would
> > be reason to factor code out into a new style-neutral UI layer - but that's
> > a whole new topic.
>
> We have the skeleton of text processing.

Huh?  An interactive text UI?

> In point of fact , I didn't use
> con2 for this example - I was making a list of general things I
> consider.
>
> > Right, that was my attempt to sell the "bool OnActivate()" style, which I
> > believe to be best. However, I do want something to get into setup *soon*,
> > so, if this hasn't convinced you, Robert, I will set about extracting the
> > minimal set of AcceptActivate() changes from Gary's monster-patch.
>
> It hasn't convinced me. You've asserted your opinion, which I
> understand, but you've not swayed me. You've not demonstrated any
> benefits I hadn't considered, and you seem to miss the long term costs
> that mixing the two functions /will/ incur - even though they have
> already affected you!
>
> I repeat the ruling I gave Gary some moons ago:
> Use a query method. It's appropriate, less code, easier to adapt in the
> future, and a cleaner design.
>

I once again leave you two to fight it out while I get some actual code written.

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