Since images do not work with pony mail, enclosed please find them as
attachments.
Best regards
Jing
On Thu, Oct 14, 2021 at 6:43 PM Jing Ge <j...@ververica.com> wrote:
> Agree. If the description contains more information than the return tag
> comment. Part of content overlap is acceptable. Otherwise I think it
> depends on the visibility and influence of the API. Agree again, it is not
> a big issue for internal interfaces and classes. But for some core public
> APIs that will be used by many application developers, such redundancy
> looks unprofessional and sometimes even confused, e.g. using read mode in
> Intellij Idea, it will look like:
>
> [image: image.png]
> [image: image.png]
>
> When I read these for the first time, I was confused, why again and again?
> Is there anything wrong with my Intellij idea, maybe the rendering has a
> bug? After I realized it was the "paragon of redundancy" - word from
> Robert C. Martin, you could imagine my feeling about the code quality at
> that time :-).
>
> I would suggest doing clean-up for some core public APIs that have an
> impact on application developers.
>
> best regards
> Jing
>
> On Thu, Oct 14, 2021 at 3:24 PM Piotr Nowojski <pnowoj...@apache.org>
> wrote:
>
>> +1 for trying to clean this up, but I'm not entirely sure in which
>> direction. Usually I was fixing this towards option 1. I agree, in your
>> example option 2. looks much better, but usually the javadoc contains a
>> bit
>> more information, not only copy/pasted @return text. For example:
>>
>> /**
>> * Send out a request to a specified coordinator and return the
>> response.
>> *
>> * @param operatorId specifies which coordinator to receive the
>> request
>> * @param request the request to send
>> * @return the response from the coordinator
>> */
>> CompletableFuture<CoordinationResponse> sendCoordinationRequest
>>
>> Sometimes this can be split quite nicely between @return and main java
>> doc:
>>
>> /**
>> * Try to transition the execution state from the current state to the
>> new state.
>> *
>> * @param currentState of the execution
>> * @param newState of the execution
>> * @return true if the transition was successful, otherwise false
>> */
>> private boolean transitionState(ExecutionState currentState,
>> ExecutionState newState);
>>
>> but that's not always the case.
>>
>> At the same time I don't have hard feelings either direction. After all it
>> doesn't seem to be that big of an issue even if we leave it as is.
>>
>> Best,
>> Piotrek
>>
>> czw., 14 paź 2021 o 14:25 Jing Ge <j...@ververica.com> napisał(a):
>>
>> > Hi Flink developers,
>> >
>> > It might be a good idea to avoid the redundant javadoc comment found in
>> > some classes, e.g. org.apache.flink.core.fs.Path w.r.t. the @Return tag
>> > comment on some methods.
>> >
>> > To make the discussion clear, let's focus on a concrete example(there
>> are
>> > many more):
>> >
>> > > /**
>> > > * Returns the FileSystem that owns this Path.
>> > > *
>> > > * @return the FileSystem that owns this Path
>> > > * @throws IOException thrown if the file system could not be
>> retrieved
>> > > */
>> > > public FileSystem getFileSystem() throws IOException {
>> > > return FileSystem.get(this.toUri());
>> > > }
>> > >
>> > >
>> > In order to remove the redundancy, there are two options:
>> >
>> > option 1: keep the description and remove the @Return tag comment:
>> >
>> > > /**
>> > > * Returns the FileSystem that owns this Path.
>> > > *
>> > > * @throws IOException thrown if the file system could not be
>> retrieved
>> > > */
>> > > public FileSystem getFileSystem() throws IOException {
>> > > return FileSystem.get(this.toUri());
>> > > }
>> > >
>> > > option 2: keep the @return tag comment and remove the duplicated
>> > description:
>> >
>> > > /**
>> > > * @return the FileSystem that owns this Path
>> > > * @throws IOException thrown if the file system could not be
>> retrieved
>> > > */
>> > > public FileSystem getFileSystem() throws IOException {
>> > > return FileSystem.get(this.toUri());
>> > > }
>> > >
>> > > It looks like these two options are similar. From the developer's
>> > perspective, I would prefer using @return tag comment, i.e. option 2.
>> > Having an explicit @return tag makes it easier for me to find the return
>> > value quickly.
>> >
>> > This issue is very common, it has been used as a Noise Comments example
>> in
>> > Uncle Bob's famous "Clean Code" book on page 64 but unfortunately
>> without
>> > giving any clear recommendation about how to solve it. From
>> Stackoverflow,
>> > we can find an interesting discussion about this issue and developer's
>> > thoughts behind it:
>> >
>> >
>> https://stackoverflow.com/questions/10088311/javadoc-return-tag-comment-duplication-necessary
>> > .
>> > Javadoc 16 provides even a new feature to solve this common issue.
>> >
>> > Since @return is recommended to use for the javadoc, I would suggest
>> Flink
>> > community following it and therefore open this discussion to know your
>> > thoughts. Hopefully we can come to an agreement about this. Many thanks.
>> >
>> > Best regards
>> > Jing
>> >
>>
>
