On Fri, 13 May 2022 08:28:50 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> A common reason for using the `TextFormatter` is the need for a `filter`.
>> Therefore, the following constructor is typically used:
>> `public TextFormatter(@NamedArg("filter") UnaryOperator<Change> filter) { 
>> ... }`
>> 
>> With that, no `valueConverter` is set in the `TextFormatter`.
>> 
>> When a `TextField` will commit his value, `TextFormatter.updateValue(...)` 
>> is called.
>> Since `valueConverter` is null, an NPE is thrown and catched inside it and 
>> `TextFormatter.updateText()` is called  (which will call 
>> `TextInputControl.updateText(...)`), which won't do anything either since 
>> `valueConverter` is still not set (=null).
>> 
>> A minor improvement here is to just skip `TextFormatter.updateValue(...)` 
>> and therefore `TextFormatter.updateText()` (-> 
>> `TextInputControl.updateText(...)`), since this methods won't do anything 
>> without a `valueConverter`.
>> With that change, no exception (NPE) is thrown and catched, and therefore 
>> also exception breakpoints are not called (can be annoying ;)).
>> This will also slightly increase the performance, as throwing exceptions is 
>> a (minor) bottleneck.
>> 
>> Added tests for all `TextFormatter` functionalities, which pass before and 
>> after.
>
> Marius Hanl has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8286552: Added space and revert typo fix

LGTM

-------------

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/794

Reply via email to