#753 - MaterialValueBox now has a new attribute called #returnBlankAsNull

[jzaruba]

With this attribute set to true calling #getvalue() returns null when the (already implemented) blank-validator recognizes the value as blank

[kevzlou7979]

Thanks for this contribution. Will write a simple test case for this.

[kevzlou7979]

Hi @jzaruba , We found some regressions with your commit from this PR #757 and we have an issue with getInternalValue() together with the addBlankItemIfNeeded() and removeBlankItemIfNeeded(). So we need your update with these regression.

[kevzlou7979]

Here's the log output.

[jzaruba]

i am gonna look into it

[jzaruba]

Hello again :)

Up to a certain point the value comparison in MaterialListValueBox.getIndex(T) was done by Objects.equals(Object, Object). Then it got changed to a NPE-prone MaterialListValueBox.getValue(int).equals(Object).
Then the MaterialListValueBoxTest.testAllowBlanks test got introduced in another branche. (At this point it had to fail upon calling MaterialListValueBox.setValue(null) if run with the new value comparing.)
Then I created those two MRs of mine. (One of those MRs changed the comparison to MaterialListValueBox.getValueInternal(int).equals(Object).)

Check out this merge a89aa2f that took place on 31.10.2017 at 00:48. This is, IMO, where the MaterialListValueBox.setAllowBlank(boolean) feature meets the NPE-prone value comparing.

When the above mentioned test invokes MaterialListValueBox.getIndex(null) we run into null.equals(Object). If we revert the comparison back to Objects.equals(Object, Object) again we should be OK.

Was there any particular reason for abandoning the null-safe Objects.equals? Or am I missing something?

Cheers
Jarda

[kevzlou7979]

Hi @jzaruba ,

Yeah between those PR I get the latest changes from yours (i.e getValueInternal) instead of the Objecs.equals(Object, Object). The test passed using the Objects.equals(Object, Object). I think we can move on to this.

Best Regards,
Mark