Implement ReadOnlyHasValue helper

[pekam]

Ported change from vaadin/framework#10643

---

This change is 

[vaadin-bot]

Make "value" transient or serializable.

[vaadin-bot]

Make "emptyValue" transient or serializable.

[denis-anisimov]

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks broke.

---

a discussion (no related file):
There is no doc changes in flow-and-components-documentation repo

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 106 at r2 (raw file):

Quoted 21 lines of code…

                        new ValueChangeEvent<V>() {
                            @Override
                            public HasValue<?, V> getHasValue() {
                                return that;
                            }
 
                            @Override
                            public boolean isFromClient() {
                                return false;
                            }
 
                            @Override
                            public V getOldValue() {
                                return oldValue;
                            }
 
                            @Override
                            public V getValue() {
                                return value;
                            }
                        });

This is very long anonymous class inside for inside if inside method.
Please extract it as a separate nested class.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 136 at r2 (raw file):

throw new IllegalArgumentException("Not Writable");

Curly brackets around.
Even though it's not in the original PR.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 142 at r2 (raw file):

throw new IllegalArgumentException("Not Writable");

Curly brackets around.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 154 at r2 (raw file):

        return emptyValue;
    }
}

new line

---

Comments from Reviewable

[pekam]

Review status: 1 of 2 files reviewed at latest revision, 7 unresolved discussions.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 106 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

                        new ValueChangeEvent<V>() {
                            @Override
                            public HasValue<?, V> getHasValue() {
                                return that;
                            }
 
                            @Override
                            public boolean isFromClient() {
                                return false;
                            }
 
                            @Override
                            public V getOldValue() {
                                return oldValue;
                            }
 
                            @Override
                            public V getValue() {
                                return value;
                            }
                        });

This is very long anonymous class inside for inside if inside method.
Please extract it as a separate nested class.

I can't because I need to implement those getters to return correct values for each case.

I extracted a method that creates and fires the event instead.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 136 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

throw new IllegalArgumentException("Not Writable");

Curly brackets around.
Even though it's not in the original PR.

Done.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 142 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

throw new IllegalArgumentException("Not Writable");

Curly brackets around.

Done.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 154 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

new line

Done.

---

Comments from Reviewable

[denis-anisimov]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 106 at r2 (raw file):

Previously, pekam (Pekka Maanpää) wrote…

I can't because I need to implement those getters to return correct values for each case.

I extracted a method that creates and fires the event instead.

I don't really see any problem here.
HasValue<ValueChangeEvent<V>, V> , oldValue , value may be provided as arguments in a CTOR for the class (in the same way as they have been provided as arguments in the fireValueChangeEvent method) .
It is always possible the question is just how many arguments you will need to pass to the CTOR.
There are only three of them here. Not that many.

If you make this class nested then there will no need on the method but instead there will be a long CTOR. In the result it's the same number of lines but it's more readable.

---

Comments from Reviewable

[pekam]

Review status: 1 of 2 files reviewed at latest revision, 4 unresolved discussions.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 106 at r2 (raw file):

Previously, denis-anisimov (Denis) wrote…

I don't really see any problem here.
HasValue<ValueChangeEvent<V>, V> , oldValue , value may be provided as arguments in a CTOR for the class (in the same way as they have been provided as arguments in the fireValueChangeEvent method) .
It is always possible the question is just how many arguments you will need to pass to the CTOR.
There are only three of them here. Not that many.

If you make this class nested then there will no need on the method but instead there will be a long CTOR. In the result it's the same number of lines but it's more readable.

Right.. Didn't realize that. Thank you! :)
Done.

---

Comments from Reviewable

[vaadin-bot]

Make "value" transient or serializable.

[vaadin-bot]

Make "oldValue" transient or serializable.

[vaadin-bot]

SonarQube analysis reported 4 issues

• 4 major

Watch the comments in this conversation to review them.

[pekam]

Review status: 1 of 2 files reviewed at latest revision, 6 unresolved discussions.

---

a discussion (no related file):

Previously, denis-anisimov (Denis) wrote…

There is no doc changes in flow-and-components-documentation repo

Done:
vaadin/flow-and-components-documentation#166

---

Comments from Reviewable

[denis-anisimov]

---

Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

flow-data/src/main/java/com/vaadin/flow/data/binder/ReadOnlyHasValue.java, line 135 at r4 (raw file):

private static class ReadOnlyValueChangeEvent<V>

I think we have some kind of agreement to keep nested class definitions on the top of the class.
I personally like those classes on the bottom. So if SQ doesn't complain then I'm OK with this as well.

---

Comments from Reviewable