On Wed, Sep 30, 2009 at 12:07 PM, Stéphane Ducasse <[email protected]> wrote: > I agree with andreas point. :)
Hi Andreas, this is an interesting discussion and I think I understand your point : you see the problem of the current API but want to avoid impacting immediately all the code base. To do that, you propose to introduce the following 4 new methods: #request:onCancel:, #request:initialAnswer:onCancel:, #multiLineRequest:centerAt:initialAnswer:answerHeight:onCancel: and #requestPassword:onCancel:. Am I correct? To me, this is introducing needless complexity. The nil value is here exactly to indicate the absence of a value. When the first implementation of the #request: method was created, I have no idea why they didn't choose to return nil upon cancel. I really have no idea how users of this library never had the need to distinguish an empty answer from a cancel. I can see so many examples where it is crucial to distinguish. Do you agree with me that the current API is broken? I'm not talking about adding a feature, I'm talking about fixing a bug. I see two options: - yours is to complicate the API in order to keep backward compatibility with the code base and thus avoiding bugs. - mine is to fix the problem once and for all. Yes, it also means bugs will appear during the period of fixing which can take some time Please let me comment on your reasons: On Tue, Sep 29, 2009 at 6:31 PM, <[email protected]> wrote: > 1) It is subtle breakage that creates a major and completely unnecessary > PITA for anyone who needs to write UIManager code that works across Pharo, > Squeak, > Croquet etc. I do not agree. Squeak doesn't have to change its implementation of #request:. Clients can just test for the nil value without wondering if it is going to be returned (in Pharo) or not (in Squeak). This has an immediate consequence : you can apply the two changesets which replace #isEmpty by #isEmptyOrNil after each call to #request:. You don't have to change #request: for this. > 2) It weakens the framework. Toolbuilder is based on having the same > semantics across > platforms and UIs and this change means that the entire family of input > requests is > no longer reliable as it will behave differently between Pharo and anything > else in > existence. To me, UIManager is broken. I don't understand why fixing it would weaken anything. Moreover, I'm quite sure all UIs can perfectly distinguish between the 2 buttons ok and cancel. For example, the Polymorph library returns nil by default and nil was translated to the empty string in PSUIManager to follow the behavior of UIManager. > 3) The alternatives are every bit as good. The protocol complication of > using > onCancel: are minimal to non-existent (since in many cases the ability to > provide a > cancel action is what you'll want anyway) and even if that were the case, a > new > protocol that over time can be supported by other UIs would solve that > completely and > in a proper evolutionary way. I agree that #onCancel: can be interesting on its own. However, I see it more as a new interesting feature whereas I'm talking about bug fixing. > 4) With the choice you've made, you need to touch every user of that method > anyway. True > If that's true, there is no penalty for changing the protocol here I don't agree. Changing the protocol means adding complexity. My solution does not complicate the protocol, it just fixes its meaning. > and it will make > migration easier since you can simply browse for all senders of #request: > and find > those that haven't been converted yet. I agree. > Please do consider these issues carefully. I really don't think there is any > advantage for you or your users by introducing such a subtle breakage. I can see pros and cons of each approach. Yours is more pragmatic I think while mine is more long-term. -- Damien Cassou http://damiencassou.seasidehosting.st "Lambdas are relegated to relative obscurity until Java makes them popular by not having them." James Iry _______________________________________________ Pharo-project mailing list [email protected] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
