setValue should not throw NPE with null

[mstahv]

Instead the component should gracefully fall back to show empty string on the field.

[Legioth]

Why?

[mstahv]

Because #FFS, the Vaadin style. I (and most likely around 99% of other users) don't want to write null checks when setting the value:

if(c.getNotes() != null) {
    textArea.setValue(c.getNotes());
}

[Legioth]

What about using the high-level Binder instead of low-level APIs?

[mstahv]

If it was easier to use (for single field bindings), I'd definitely use that.

[mstahv]

Why is this flagged as a breaking change? I think it might actually be fixing some apps that are broken in certain situtations ;-)

[pleku]

Why is this flagged as a breaking change? I think it might actually be fixing some apps that are broken in certain situtations ;-)

Because it is a breaking change if we change the semantics to now start to accept null and start to do something else instead. To be honest, I don't see us doing that. BUT what we probably should do is the Single Field Binding that we are currently missing starting from 8.

[mstahv]

I agree some kind of single field binging thingie would be nice, for example to add EmailValidator to a TextField.

But I didn't yet realise why throwing NPE would be semantically (or in any ways) better than just clearing the value. I also don't believe that there is anyone relying on the fact that it is currently thrown. If a developer sets a text field to null (directly or from another objects property), they sure mean to clear the field.

[Legioth]

One problem is that accepting setValue(null) implies that getValue() should then also return null. Making getValue() return something other than what was passed to setValue() is just weird.

And allowing getValue() to return null is a gain a whole new world of potential compatibility issues.

[mstahv]

Maybe there should be an API that you could use to declare if empty string is interpreted as null ;-)

Seriously, I wouldn't take this so academically. I have hard time seeing a real world use case where it would be problem that getValue would return "" even though setValue(null) was just called. At least those would be far more rare than the problems we are currently causing (UI gets totally broken if some property suddenly is null and code gets more complex if you tackle this in application code).

We should be fighting for simplicity in our users code, not in frameworks.

[Legioth]

Yes, UI can get broken if you are not testing properly. That's the case regardless of what we do.

There's more to simplicity than how many lines of code is needed, e.g. how easy it is for someone to understand what the code does.

[mstahv]

The simplicity with this change would mean that end users would need to write less code, whether they did extensive tests or not. I don't believe anyone would have more problems to figure out what setValue does.

[nemojmenervirat]

I think that this NPE makes life hard. I don't use setValue directly i use binder and readBean or setBean. If I have null in field with type String it works OK, but if I have nulls in fields like (Long, BigDecimal,...) i have to init them before calling readBean/setBean. It was not expected behavior because i have nulls in database in some columns and I want to init edit form. If user just opens edit form, changes nothing and clicks Confirm button i have to do extra work to avoid updating data (bigDecimalValue and longValue should stay null in database not zero)

class MyBean {
        private BigDecimal bigDecimalValue;
        private Long longValue;
        private String stringValue;

        public BigDecimal getBigDecimalValue() {
            return bigDecimalValue;
        }

        public void setBigDecimalValue(BigDecimal bigDecimalValue) {
            this.bigDecimalValue = bigDecimalValue;
        }

        public Long getLongValue() {
            return longValue;
        }

        public void setLongValue(Long longValue) {
            this.longValue = longValue;
        }

        public String getStringValue() {
            return stringValue;
        }

        public void setStringValue(String stringValue) {
            this.stringValue = stringValue;
        }

    }

    public MainView() {
        Binder<MyBean> binder = new Binder<>();

        TextField textFieldBigDecimal = new TextField("Big decimal value");
        add(textFieldBigDecimal);
        TextField textFieldLongValue = new TextField("Long value");
        add(textFieldLongValue);
        TextField textFieldStringValue = new TextField("String value");
        add(textFieldStringValue);

        binder.forField(textFieldBigDecimal).withConverter(new StringToBigDecimalConverter("Not a big decimal")).bind(MyBean::getBigDecimalValue,
                MyBean::setBigDecimalValue);
        binder.forField(textFieldLongValue).withConverter(new StringToLongConverter("Not a long")).bind(MyBean::getLongValue, MyBean::setLongValue);
        binder.forField(textFieldStringValue).bind(MyBean::getStringValue, MyBean::setStringValue);

        MyBean myBean = new MyBean();

        // this is required to avoid NPE
        myBean.setBigDecimalValue(BigDecimal.ZERO);
        myBean.setLongValue(0l);
        // null String is ok

        binder.setBean(myBean);

    }