Thanks for driving this thread Andrey. Conclusion looks good to me.
Best,
tison.
Andrey Zagrebin 于2019年8月21日周三 下午7:25写道：
> FYI the PR: https://github.com/apache/flink-web/pull/251
> A review from the discussion participants would be appreciated.
>
> On Wed, Aug 21, 2019 at 1:23 PM Timo Walther
FYI the PR: https://github.com/apache/flink-web/pull/251
A review from the discussion participants would be appreciated.
On Wed, Aug 21, 2019 at 1:23 PM Timo Walther wrote:
> Thanks for summarizing the discussion Andrey, +1 to this style.
>
> Regards,
> Timo
>
>
> Am 21.08.19 um 11:57 schrieb An
Thanks for summarizing the discussion Andrey, +1 to this style.
Regards,
Timo
Am 21.08.19 um 11:57 schrieb Andrey Zagrebin:
Hi All,
It looks like we have reached a consensus regarding the last left question.
I suggest the following final summary:
- Use @Nullable annotation where you do
Hi All,
It looks like we have reached a consensus regarding the last left question.
I suggest the following final summary:
- Use @Nullable annotation where you do not use Optional for the
nullable values
- If you can prove that Optional usage would lead to a performance
degradation i
Thanks for the summarize Andrey!
I'd also like to adjust my -1 to +0 on using Optional as parameter for
private methods due to the existence of the very first rule - "Avoid using
Optional in any performance critical code". I'd regard the "possible GC
burden while using Optional as parameter" also
I think Dawid raised a very good point here.
One of the outcomes should be that we are consistent in our recommendations
and requests during PR reviews. Otherwise we'll just confuse contributors.
So I would be
+1 for someone to use Optional in a private method if they believe it is
helpful
-1
Hi Andrey,
Just wanted to quickly elaborate on my opinion. I wouldn't say I am -1,
just -0 for the Optionals in private methods. I am ok with not
forbidding them there. I just think in all cases there is a better
solution than passing the Optionals around, even in private methods. I
just hope the
For the use of optional in private methods: It sounds fine to me, because
it means it is strictly class-internal (between methods and helper methods)
and does not leak beyond that.
On Mon, Aug 19, 2019 at 5:53 PM Andrey Zagrebin
wrote:
> Hi all,
>
> Sorry for not getting back to this discussion
Hi all,
Sorry for not getting back to this discussion for some time.
It looks like in general we agree on the initially suggested points:
- Always use Optional only to return nullable values in the API/public
methods
- Only if you can prove that Optional usage would lead to a
pe
Hi Qi,
> For example, SingleInputGate is already creating Optional for every
> BufferOrEvent in getNextBufferOrEvent(). How much performance gain would we
> get if it’s replaced by null check?
When I was introducing it there, I have micro-benchmarked this change, and
there was no visible throu
I'd be in favour of
- Optional for method return values if not performance critical
- Optional can be used for internal methods if it makes sense
- No optional fields
Cheers,
Till
On Mon, Aug 5, 2019 at 12:07 PM Aljoscha Krettek
wrote:
> Hi,
>
> I’m also in favour of using Optional only for me
Hi,
I’m also in favour of using Optional only for method return values. I could be
persuaded to allow them for parameters of internal methods but I’m sceptical
about that.
Aljoscha
> On 2. Aug 2019, at 15:35, Yu Li wrote:
>
> TL; DR: I second Timo that we should use Optional only as method r
TL; DR: I second Timo that we should use Optional only as method return
type for non-performance critical code.
>From the example given on our AvroFactory [1] I also noticed that Jetbrains
marks the OptionalUsedAsFieldOrParameterType inspection as a warning. It's
relatively easy to understand why
Hi,
First, Optional is just a wrapper, just like boxed value. So as long as it's
not a field level operation, I think it is OK to performance.
I think guava optional has a good summary to the uses. [1]
> As a method return type, as an alternative to returning null to indicate
that no value was a
Hi everyone,
I would vote for using Optional only as method return type for
non-performance critical code. Nothing more. No fields, no method
parameters. Method parameters can be overloaded and internally a class
can work with nulls and @Nullable. Optional is meant for API method
return types
Hi Jark & Zili,
I thought it means "Optional should not be used for class fields". However
now I get a bit confused about the edited version.
Anyway +1 to "Optional should not be used for class fields"
Thanks,
Biao /'bɪ.aʊ/
On Fri, Aug 2, 2019 at 5:00 PM Zili Chen wrote:
> Hi Jark,
>
> Foll
Hi Zili,
Yes. I agree to use @Nullable/@Nonnull/SerializableOptional as the class
field instead of Optional.
On Fri, 2 Aug 2019 at 17:00, Zili Chen wrote:
> Hi Jark,
>
> Follow your opinion, for class field, we can make
> use of @Nullable/@Nonnull annotation or Flink's
> SerializableOptional.
Hi Jark,
Follow your opinion, for class field, we can make
use of @Nullable/@Nonnull annotation or Flink's
SerializableOptional. It would be sufficient.
Best,
tison.
Jark Wu 于2019年8月2日周五 下午4:57写道：
> Hi Andrey,
>
> I have some concern on point (3) "even class fields as e.g. optional config
> o
Hi Andrey,
I have some concern on point (3) "even class fields as e.g. optional config
options with implicit default values".
Regarding to the Oracle's guide (4) "Optional should not be used for class
fields".
And IntelliJ IDEA also report warnings if a class field is Optional,
because Optional i
Hi Andrey,
Thanks for working on this.
+1 it's clear and acceptable for me.
To Qi,
IMO the most performance critical codes are "per record" code path. We
should definitely avoid Optional there. For your concern, it's "per buffer"
code path which seems to be acceptable with Optional.
Just one m
Agree that using Optional will improve code robustness. However we’re
hesitating to use Optional in data intensive operations.
For example, SingleInputGate is already creating Optional for every
BufferOrEvent in getNextBufferOrEvent(). How much performance gain would we get
if it’s replaced by
Hi Andrey,
I think that’s a good compromise easy to follow, so +1 from my side.
Piotrek
> On 1 Aug 2019, at 18:00, Andrey Zagrebin wrote:
>
> EDIT: for Optional in public API vs performance concerns
>
> Hi all,
>
> This is the next follow up discussion about suggestions for the recent
> thre
EDIT: for Optional in public API vs performance concerns
Hi all,
This is the next follow up discussion about suggestions for the recent
thread about code style guide in Flink [1].
In general, one could argue that any variable, which is nullable, can be
replaced by wrapping it with Optional to ex
23 matches
Mail list logo
