RR: HasValue is not ready for primetime

[Ray Ryan]

As much as I like the spirit HasValue interface introduced with the new event handlers, I think we've rushed it, and that it should be removed for 1.6. It stands a good chance of interfering with nascent data binding / validation work. And its application to CheckBox is just plain wrongheaded and confusing. (CheckBox#setValue doesn't actually set the input element's value attribute--oops.)

I'd like to take it out. The CL to do this will:

• Delete the HasValue interface
  
• Leave DatePicker as is
  
• Delete TextBoxBase#setValue(String), and add rename TextBoxBase#setValue(String, Boolean) to setText(String, Boolean)
  
• Delete CheckBox#SetValue, and make it stop throwing ValueChange events.

Reactions?

rjrjr

[Isaac Truett]

> Reactions?

Puzzlement.

> It stands a good chance of interfering with nascent data binding /
> validation work.

Care to elaborate on this?

> And its application to CheckBox is just plain wrongheaded
> and confusing. (CheckBox#setValue doesn't actually set the input element's
> value attribute--oops.)

When has GWT's CheckBox ever had a "value" in the HTML sense? As a
Widget, it represents a boolean value.

Obviously you're not telling the whole story here.

[Ray Ryan]

On Thu, Dec 11, 2008 at 12:28 PM, Isaac Truett <itr...@gmail.com> wrote:

> Reactions?

Puzzlement.

> It stands a good chance of interfering with nascent data binding /
> validation work.

Care to elaborate on this?

We'll need a general stick-a-value-in-take-a-value-out mechanism, the HasValue interface may not be the right one, and we'd prefer not to lead with it in isolation from the framework as a whole. 

> And its application to CheckBox is just plain wrongheaded
> and confusing. (CheckBox#setValue doesn't actually set the input element's
> value attribute--oops.)

When has GWT's CheckBox ever had a "value" in the HTML sense? As a
Widget, it represents a boolean value.

Yes, and the lack of access to the  underlying value attribute has long been confusing to folks with an HTML background. Adding a setValue method that some have considered to be missing, but having it not apply to the checkbox value is just plain confusing.

Obviously you're not telling the whole story here.

Well, no, really that's it.

[Isaac Truett]

> We'll need a general stick-a-value-in-take-a-value-out mechanism, the
> HasValue interface may not be the right one, and we'd prefer not to lead
> with it in isolation from the framework as a whole.

If HasValue isn't a contract that you want to honor, that's one thing.
But saying that it would interfere with a data binding or validation
framework just doesn't seem accurate. Even if you end up not using
HasValue as part of those frameworks, it isn't a loss. It's just not
used. HasValue consolidates all of the separate accessors that Widgets
have to define anyway and gives them a common API. That's not stepping
on any other API or framework's toes.

> Yes, and the lack of access to the underlying value attribute has long been
> confusing to folks with an HTML background. Adding a setValue method that
> some have considered to be missing, but having it not apply to the checkbox
> value is just plain confusing.

I don't necessarily agree with this position, but I can accept that
others might. So punt on CheckBox for now. Nothing says every Widget
has to implement HasValue. Do you actually anticipate finding a
replacement for HasValue that will fit every current and future Widget
perfectly?

[Freeland Abbott]

Without having actually looked at HasValue much, I did get pinged by a friend of mine who was annoyed that his overridden parameterized getValue() was now colliding with our String getValue()... looking at CheckBox, perhaps something like HasValue<Boolean> would be a better semantic.  (It also might work better for my friend's case, which was basically a parameterized parent for IntegerTextBox, CurrencyTextBox, TelephoneTextBox, etc.)

I think Ray's point is just that we'd rather not have GWT wind up tied to something half-thought-out, but have to support it three years from now as well as whatever we discover we want instead.  Fearing that we just stepped onto such a route, he's trying to step back before it's too late and he's stuck.

It'd be nice if such a thing could drop into the incubator to bake, of course. <grin> 

[Isaac Truett]

The HasValue interface's usefulness is entirely separable from any
framework or library which might or might not be built on top of it.
TelephoneTextBox is a great example of that. You could wrap a single
TextBox in a composite that implements HasValue<TelephoneNumber> and
another composite (TelephoneChooser?) that wraps three TextBoxes (Area
Code, Exchange, and... whatever you call the last four numbers). Yet
another implementation might use DropDownListBox to limit the
available area codes (to area codes in a given state, for example).
All of those can be treated as HasValue<TelephoneNumber> and have
consistent, type-safe accessors defined by that interface.

Not every Widget needs to implement HasValue. Widgets could be
retrofitted later. And HasValue doesn't have to replace current
accessors. TextBox.getText() could trivially delegate to getValue()
and wouldn't even need to be deprecated. There is the potential for
collision with any existing Widget extensions that define methods
called getValue() or setValue(). That's unfortunate, but unavoidable
when adding to an existing API. That should also be an understood risk
when extending a class from another source. If you want to protect
against changing API, encapsulation is much safer.

I cannot agree more with the cautious spirit of this proposal, but I
don't think unquantified fears about future data binding and
validation work justify holding this up. HasValue did a brief stint in
incubation and maybe it's been called up prematurely, but I don't
think so. Consider HasValue on its own merits.

[Freeland Abbott]

To be fair, my friend was extending TextBox---which came to implement HasValue, and thus acquired the colliding String getValue()---when he should have extended Composite (which doesn't) instead; that was my suggested resolution for him.  He grumbled ("but it 'is-a' TextBox, that should be extends"), but conceded.

However, the "old" HasValue is not parameterized, and implies something has *string* value, period.  As applied to CheckBox, that's confusing-to-wrong.  

Isaac is correct that we can resolve this by making CheckBox not a HasValue, and keep the interface... but the discussion makes me think that HasValue<T> has merit, and for example a CheckBox would be HasValue<Boolean> and a TextBox would be HasValue<String> (my friend should still make Composites, with HasValue<TelephoneStationNumber>, and not extend TextBox).

But if we go with HasValue (unparameterized), that's a breaking change to make later (I guess we'd have to make HasTypedValue<T> instead, and have HasValue come to extend HasTypedValue<String>), and so I'm still happy to pause and think a bit more.

(Do we like HasValue<T>, where T is widget-specific for whatever widgets have values??)

[Isaac Truett]

Ah. We have a disconnect:

> However, the "old" HasValue is not parameterized, and implies something has
> *string* value, period. As applied to CheckBox, that's confusing-to-wrong.

I'm not familiar with the old HasValue. The code I was referring to
was originally committed to gwt-incubator as r1093 on Oct. 8 and was
included in the 1_6_DatePicker branch of GWT. It is parameterized as
you suggest. HasValue without a parameterized type would indeed by
awkward (and essentially what HasText is now). I believe that in the
1_6_DatePicker branch TextBoxBase implements HasValue<String> which
would explain how TextBox comes to have a String getValue() method.

CheckBox implements HasValue<Boolean> makes sense to me. I can see the
other side (CheckBox's "value" is a String as per HTML and the boolean
state is separate). I would argue that the HTML value of a CheckBox is
largely irrelevant in an AJAX application, but I can't back that up
with anything other than personal experience. Ray has people ready to
argue the opposite, so it's a wash there.







On Thu, Dec 11, 2008 at 2:20 PM, Freeland Abbott

[John Tamplin]

On Thu, Dec 11, 2008 at 2:20 PM, Freeland Abbott <gwt.team...@gmail.com> wrote:

To be fair, my friend was extending TextBox---which came to implement HasValue, and thus acquired the colliding String getValue()---when he should have extended Composite (which doesn't) instead; that was my suggested resolution for him.  He grumbled ("but it 'is-a' TextBox, that should be extends"), but conceded.

However, the "old" HasValue is not parameterized, and implies something has *string* value, period.  As applied to CheckBox, that's confusing-to-wrong.  

Isaac is correct that we can resolve this by making CheckBox not a HasValue, and keep the interface... but the discussion makes me think that HasValue<T> has merit, and for example a CheckBox would be HasValue<Boolean> and a TextBox would be HasValue<String> (my friend should still make Composites, with HasValue<TelephoneStationNumber>, and not extend TextBox).

But if we go with HasValue (unparameterized), that's a breaking change to make later (I guess we'd have to make HasTypedValue<T> instead, and have HasValue come to extend HasTypedValue<String>), and so I'm still happy to pause and think a bit more.

(Do we like HasValue<T>, where T is widget-specific for whatever widgets have values??)

One problem is that due to type erasure you cannot implement multiple copies of this interface (such as HasValue<Boolean> and HasValue<String>).  For a hypothetical checkbox, you might well want a boolean value for checked or not, as well as the string value (ie, what it would post as in a form submit).

Other than that, I think the generic version is the way to go if it is kept.

--
John A. Tamplin
Software Engineer (GWT), Google

[Isaac Truett]

At the risk of seeming to hand-wave that problem away, I would say
that any Widget seeking to implement HasValue twice is not a candidate
for HasValue at all. HasValue is, by definition, for Widgets with a
single distinct value. The value of a CheckBox is either a String or a
Boolean (we've seen arguments either way) or it simply isn't a
HasValue because it's a complex Widget with two equally important and
independent values.

[Ray Cromwell]

IMHO, the concept of a Widget having a value is that it has only one
value. The "value" can certainly be an object with multiple fields, or
with conversion helper functions (String->Boolean), but it's one value
with a normalized internal representation.

For example, consider an USPostalAddressWidget, which
HasValue<USPostalAddress>. This widget is quite complex, with probably
ample internal textboxes for each field (street, city, state, zip,
suite, etc) and also validation. However, the widget conceptually has
a single value: a USPostalAddress that can be get and set.

In this context, I think it is clear that HasValue is really a
Model-View interface. If it were a Table, it would be
HasValue<TableModel> or HasValue<TableData>, for example. Probably the
primary difference is HasValue doesn't require models to provide
change listener interfaces.

I don't believe that Data Binding and Model/View architectures are
mutually exclusive. The main difference is, with MVC, typically, you
build model POJOs, and build the View to be tightly coupled to the
model interface, so that there is usually (but not always) a one to
one correspondence between View and Model types. With Data Binding
frameworks, the coupling is looser and you can have a many-to-one
relationship, e.g bind POJO.fooField to Widget A and bind
POJO.barField to WidgetB, or further, you can bind into composite
widgets exposing their innards. Data Binding allows for more reuse
since you can repurpose pojo models by slicing and dicing them with
binding expressions, on the other hand, lots of bindings can get messy
and make it harder to track what widgets deal with what data,
especially if the bindings are dynamic or in a DSL.

-Ray

[Ray Ryan]

Well put, RayC, thanks. MVC is exactly the intended point of HasValue. (Freeland, John, it's HasValue<T>, that's not being debated, fret ye not.) 

And you guys are right, I think, that we're wrong to paralyze ourselves with fears of future confusion with our still-vague-but-crystallizing-nicely notions of a data binding framework.

The specific issue that got me to lob this grenade was a disgusted internal complaint by someone who has long been frustrated by our CheckBox's lack of access to the dom value field. When he saw the longed for method appear and do exactly the wrong thing, and in a manner redundant with isChecked...

I like HasValue, but its application to CheckBox has produced very confusing little beastie. So, how about we narrow the scope of this conversation to CheckBox api repair. How about this:

• Deprecate CheckBox#isChecked and CheckBox#setChecked
• Introduce CheckBox#setFormValue
• Add copious javadoc to compare and contrast the two set...Value methods

If this flies, are there other widgets that need the setFormValue method?

rjrjr

[Ray Cromwell]

For what it's worth RayR (damn it, I used to love the fact that my
name was rare on lists. Curse Google for hiring Ray's :) ), I think
you were still right to be somewhat cautious here, due to the fact
that stuff introduced into core becomes legacy and hard to change
without breakage. There might naturally be some coupling with data
binding to consider, like if the DB framework can leverage Widgets
that have the HasValue interface, etc or whether something more
complex like BeanInfo-style metadata about internal fields might be
appropriate. It does merit discussion, so it was worth bringing it up.

I would say perhaps, include HasValue in the trunk, let people get
experience with it, and it might evolve before 1.6, and if it doesn't,
it could be left in trunk, and removed from the 1.6 release. We
probably won't know the best interface until a bunch of code is
written to use it.

-Ray

[Isaac Truett]

> how about we narrow the scope of this conversation to
> CheckBox api repair. How about this:
>
> Deprecate CheckBox#isChecked and CheckBox#setChecked
> Introduce CheckBox#setFormValue
> Add copious javadoc to compare and contrast the two set...Value methods

No objections here. While we're being cautious, here's an alternate
approach that is even more guarded:

* Leave is/setChecked as-is.
* Add get/setFormValue.
* Don't implement HasValue in CheckBox at all. After HasValue gets
more exposure in trunk and GWT 1.6, solicit feedback as to whether
CheckBox would benefit from implementing it or just add confusion.

I don't propose that this is necessarily a better solution, just food
for thought.

[Arthur Kalmenson]

We're currently using the MVC approach with our project and it seems
to be working out pretty well. We follow the same approach that Ray
Cromwell mentioned with the USPostalAddress example and we use the
HasValue<T> interface to implement it. It seems to work pretty well at
the moment. Having most of the Widgets implement HasValue<T> would
really help us clean up the code.

Perhaps CheckBox should implement HasValue<T> using some "complex"
object that combines the Boolean and String value that the CheckBox
contains for its type?

--
Arthur Kalmenson