+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
>
