I opened issue #3409 for the Suggestion change and issue #3410 for the HasValue change.
http://code.google.com/p/google-web-toolkit/issues/detail?id=3409 http://code.google.com/p/google-web-toolkit/issues/detail?id=3410 - Isaac On Wed, Feb 25, 2009 at 9:07 AM, Arthur Kalmenson <[email protected]> wrote: > >> On a side note, I found when I was writing this patch that HasValue >> extends HasValueChangeHandlers in trunk. It occurs to me that this >> relationship could possibly be backwards. I don't think that something >> with a value necessarily should be required to broadcast changes. See >> the implementation of MultiWordSuggestion for an illustration of this. >> Requiring something that HasValueChangeHandlers to have a value (that >> value which changes) makes more sense to me on the face of it. > > This makes more sense to me too. > >> p.s. Thank you, Gmail Labs, for reminding me that I forgot to attach the >> patch. > > Handy, ain't it? :) > > -- > Arthur Kalmenson > > > > On Tue, Feb 24, 2009 at 6:42 PM, Isaac Truett <[email protected]> wrote: >> I basically agree with John and Ray. In general, I agree that using >> the most remote parent type possible (without introducing casts) is >> ideal. But when the subtype exists specifically as a convenience, >> using the parameterized super class instead and then complaining about >> it is... let's call it silly. >> >> To better illustrate my proposal, I have attached a quick-and-dirty >> patch against trunk r4850. In this patch Suggestion extends HasValue >> and TypedSuggestBox is a super class of SuggestBox, as I described >> above. >> >> On a side note, I found when I was writing this patch that HasValue >> extends HasValueChangeHandlers in trunk. It occurs to me that this >> relationship could possibly be backwards. I don't think that something >> with a value necessarily should be required to broadcast changes. See >> the implementation of MultiWordSuggestion for an illustration of this. >> Requiring something that HasValueChangeHandlers to have a value (that >> value which changes) makes more sense to me on the face of it. >> >> - Isaac >> >> p.s. Thank you, Gmail Labs, for reminding me that I forgot to attach the >> patch. >> >> >> On Tue, Feb 24, 2009 at 1:03 PM, Emily Crutcher <[email protected]> wrote: >>> If that concern doesn't seem like it would be a problem, then I definitely >>> agree with you that creating abstract base classes that have the >>> parametrized types seems like the best solution. >>> >>> >>> On Tue, Feb 24, 2009 at 10:54 AM, Ray Ryan <[email protected]> wrote: >>>> >>>> That feedback sounds a bit pedantic and impractical to me. And my job >>>> title used to be Senior Pedant. >>>> >>>> On Tue, Feb 24, 2009 at 7:44 AM, Emily Crutcher <[email protected]> wrote: >>>>> >>>>> It could work, though I found when I used this technique with >>>>> DatePicker (DatePicker extends AbstractDatePicker<MonthSelector, >>>>> CalandarView>), there was some feedback that having that abstract type >>>>> layer >>>>> was slightly confusing because good OO practice implied that references >>>>> should then be typed as AbstractDatePicker, which then brought in the >>>>> complexity of the generic types back into the lives of the 90% who did not >>>>> care about the parameterized arguments. >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> On Tue, Feb 24, 2009 at 10:00 AM, Ray Ryan <[email protected]> wrote: >>>>>> >>>>>> How about extracting a parameterized super class: >>>>>> AbstractSuggestionBox<T extends Suggestion> >>>>>> SuggestionBox extends AbstractSuggestionBox<Suggestion> >>>>>> rjrjr >>>>>> >>>>>> On Mon, Feb 23, 2009 at 7:15 PM, Emily Crutcher <[email protected]> wrote: >>>>>>> >>>>>>> >>>>>>> On Mon, Feb 23, 2009 at 7:04 PM, Isaac Truett <[email protected]> >>>>>>> wrote: >>>>>>>> >>>>>>>> The API documentation has this to say on the subject: >>>>>>>> >>>>>>>> "[...] To send back a DTO with each suggestion, extend the Suggestion >>>>>>>> interface and define a getter method that has a return value of the >>>>>>>> DTO's type. Define a class that implements this subinterface and use >>>>>>>> it to encapsulate each suggestion. >>>>>>>> >>>>>>>> To access a suggestion's DTO when the suggestion is selected, add a >>>>>>>> SuggestionHandler to the SuggestBox (see SuggestBox's documentation >>>>>>>> for more information). In the >>>>>>>> SuggestionHandler.onSuggestionSelected(SuggestionEvent event) method, >>>>>>>> obtain the selected Suggestion object from the SuggestionEvent object, >>>>>>>> and downcast the Suggestion object to the subinterface. Then, acces >>>>>>>> the DTO using the DTO getter method that was defined on the >>>>>>>> subinterface." >>>>>>>> >>>>>>>> See >>>>>>>> http://google-web-toolkit.googlecode.com/svn/javadoc/1.5/com/google/gwt/user/client/ui/SuggestOracle.Suggestion.html >>>>>>>> >>>>>>>> (the 1.6 version is similar, but with the new event model) >>>>>>>> >>>>>>>> So the endorsed solution is to extend and cast. Fair enough. This >>>>>>>> probably dates from pre-1.5, and it was good enough for then. But is >>>>>>>> there a reason not to parameterize SuggestBox with <T extends >>>>>>>> Suggestion> (and SuggestOracle<T>, SelectionEvent<T>, etc.) now that >>>>>>>> that's an option? Or perhaps make Suggestion implement HasValue<T>? I >>>>>>>> have an application that uses many SuggestBoxes and many different >>>>>>>> Suggestion subclasses and this would simplify things (and eliminate >>>>>>>> much type-casting). >>>>>>> >>>>>>> I'm not sure parameterizing SuggestBox itself would be worth it, as >>>>>>> most people use it without creating new types of suggestions: so the >>>>>>> parameterization would add clutter for the many and prevent casts for >>>>>>> the >>>>>>> few. Though, I completely agree it is annoying to have to cast. >>>>>>> Perhaps we >>>>>>> could create a composite-based CustomSuggestBox that is parameterized? >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Any thoughts on this? Horrible side effects that I'm missing? >>>>>>>> >>>>>>>> - Isaac >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> "There are only 10 types of people in the world: Those who understand >>>>>>> binary, and those who don't" >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> "There are only 10 types of people in the world: Those who understand >>>>> binary, and those who don't" >>>>> >>>>> >>>> >>>> >>>> >>> >>> >>> >>> -- >>> "There are only 10 types of people in the world: Those who understand >>> binary, and those who don't" >>> >>> > >>> >> >> > >> > > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
