Set response status based on server handling logic status.

[denis-anisimov]

Fixes vaadin/vaadin-upload-flow#145

---

This change is 

[joheriks]

Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @denis-anisimov)

---

flow-server/src/main/java/com/vaadin/flow/server/communication/StreamReceiverHandler.java, line 95 at r1 (raw file):

    enum UploadStatus {
        OK, ERROR
    }

Not sure about the purpose of this enum. It is only used in the return type of streamToReceiver. Seems it could be simpler and cleaner to use Boolean throughout for the upload status.

[denis-anisimov]

Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @joheriks)

---

flow-server/src/main/java/com/vaadin/flow/server/communication/StreamReceiverHandler.java, line 95 at r1 (raw file):

Previously, joheriks (Johannes Eriksson) wrote…

Not sure about the purpose of this enum. It is only used in the return type of streamToReceiver. Seems it could be simpler and cleaner to use Boolean throughout for the upload status.

Simpler: yes.
Cleaner: not really in this specific situation.

I'm trying to avoid potential mistakes.
Yes, in fact this is just boolean.
But using boolean in this case means that the return type of the method will be Pair<Boolean,Boolean>.
And the first boolean is almost semantically the same the second boolean .
The method names don't help you to see semantical difference.
It's too easy to make a mistake and use another boolean value instead of the required one.
In fact you don't even know which value to use without go through all parts of the code.

So having Pair<Boolean,Boolean> is worse than having a method operation(boolean one, boolean two) where you also don't know which semantic is for the first arg and which is for the second. But in this case you may at least give more proper names for the args and add javadocs (in fact it's better to avoid such methods at all).

I'm trying to solve this semantic issue via typesafe way.
You don't have a chance to make a mistake with the current approach.

[joheriks]

Reviewed 2 of 2 files at r1.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained

---

flow-server/src/main/java/com/vaadin/flow/server/communication/StreamReceiverHandler.java, line 95 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Simpler: yes.
Cleaner: not really in this specific situation.

I'm trying to avoid potential mistakes.
Yes, in fact this is just boolean.
But using boolean in this case means that the return type of the method will be Pair<Boolean,Boolean>.
And the first boolean is almost semantically the same the second boolean .
The method names don't help you to see semantical difference.
It's too easy to make a mistake and use another boolean value instead of the required one.
In fact you don't even know which value to use without go through all parts of the code.

So having Pair<Boolean,Boolean> is worse than having a method operation(boolean one, boolean two) where you also don't know which semantic is for the first arg and which is for the second. But in this case you may at least give more proper names for the args and add javadocs (in fact it's better to avoid such methods at all).

I'm trying to solve this semantic issue via typesafe way.
You don't have a chance to make a mistake with the current approach.

Ok, fair point (one could for consistency then consider using this enum as the return value of each of the methods that now return boolean, but that is not really important).