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] |
Responding out of thread, as I want to make a generic point about what API changes in setup will be approved, and what won't be, and the process I use internally when reviewing one - like the OnActivate change some time ago. setup has an improving design. We are slowly separating processing from user control, and feedback likewise. We have our own mini-framework abstracting some of the more heinous windowism's. Internal API changes that help in this process, that: * Factor out conflated functionality * Step us toward some form of MVC pattern * Improve readability Are likely to be accepted. Internal API changes that don't help, that is, that: * Add additional logic to an extant method. * Conflate GUI logic with processing logic. * Make setup harder to read. Are unlikely to be accepted. That said, lets examine the void OnActivate () -> bool OnActivate change. I asked Gary to come up with something after I hacked up the antivirus disabling page.. what he came up with identified the best place to be making the change, but not the best way... void OnActivate is an event handler: it handles an event - the idiom for which is well known: no return values are given, any errors are propogated via raised exceptions or a global error handling logic for the application, or logged on the page the event handler is part of. We need a method to have some pages not displayed. Good modular design allows that the choice to show or not show the page should: * Be made by the page where the page has the information, * Be made outside the page where some other aspect of the app has the knowledge. In this case, the virus scanning page is the only part of the GUI that knows whether it should be shown: so the choice to show or not show the page should reside with the page. OnCreate must fire, or the page won't exist to make the choice. What we really want is to prevent page activation: so something on or near OnActivate is appropriate. So far, so good. Now, Gary extended OnActivate to return success or failure. This means that /every/ overriden OnActivate has to decide whether the page will be shown or not. It's not an optional part of the API. Alternatives? A query method on the page asking whether it wants to be displayed? A static somewhere the page's Create method can set? some other approach. A query method has several advantages over changing OnActivate's meaning. * It doesn't confuse the idiom. * As a separate method, it can be given an intuitive name, * As a separate method, it can be given a reasonable default, reducing programmer work. So it allows us to achieve two of the pro points waaay above, and none of the con points. Changing OnActivates meaning would conflate things in the method, and make the code harder to read - "what does false mean - that Activate failed?". Cheers, Rob -- GPG key available at: <http://members.aardvark.net.au/lifeless/keys.txt>.
Attachment:
signature.asc
Description: This is a digitally signed message part
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |