You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by "Sven Meier (JIRA)" <ji...@apache.org> on 2011/01/03 00:30:47 UTC

[jira] Commented: (WICKET-3269) Review AbstractTextComponent's handling of empty Strings in 1.5

    [ https://issues.apache.org/jira/browse/WICKET-3269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976576#action_12976576 ] 

Sven Meier commented on WICKET-3269:
------------------------------------

I'll gladly continue this discussion on dev@.

But before I do so, I'd like to ask you to take a final look at the attached "emptyStringsToNullConversion.patch" and then reconsider your decision.

The patch doesn't change empty string to null handling, it just moves it into the right location: the converter.

IMHO the change in ConverterLocator (note the adjusted assert in ConvertersTest) proves my point on why this special handling should be removed from AbstractTextComponent and be handled in a more appropriate location. The amount of removed code clearly speaks for itself.

(( Secondary effect:
I can change the conversion in my applications as I like, even if I'm only the 1% usecase ;).
public class Foo {
  public void setBar(String bar) {
    assert bar != null;
    this.bar = bar;
  }
}
If you have this all over the place, you don't want to tell every TextField to leave empty strings empty.
))

> Review AbstractTextComponent's handling of empty Strings in 1.5
> ---------------------------------------------------------------
>
>                 Key: WICKET-3269
>                 URL: https://issues.apache.org/jira/browse/WICKET-3269
>             Project: Wicket
>          Issue Type: Improvement
>          Components: wicket
>    Affects Versions: 1.5-M3
>            Reporter: Sven Meier
>            Assignee: Igor Vaynberg
>         Attachments: AbstractTextComponent_A.diff, AbstractTextComponent_B.diff, WICKET-3269-test.patch, WICKET-3269.patch
>
>
> Context:
> With convertEmptyInputStringToNull set to 'true' (the default), AbstractTextComponent converts empty strings to 'null'.
> This works only if the component doesn't know its model type.
> Problem:
> In our application we would like to always keep empty Strings as they are. We tried to use a ComponentInstantiationListener to set convertEmptyInputStringToNull to 'false', but regretfully AbstractTextComponent reverts this setting to 'true' in its constructor.
> Proposals for 1.5 (either A or B):
> A) Change default of convertEmptyInputStringToNull to 'false'.
> - allows applications to keep empty strings as they are,
> - if needed, clients can use a ComponentInstantiationListener to globally set convertEmptyInputStringToNull to 'true' for the old default
> B) Remove support for special handling of empty strings.
> - no special handling in #convertValue(String[]) and #resolveType() needed,
> - no more ambiguity whether ConvertEmptyInputStringToNull has any effect (model type known or not),
> - if needed, clients can use a custom Converter to convert empty strings to 'null'.
> See attached patches, note that no unit tests changes were required.
> Thanks for your consideration.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.