Attachments don't work on this mailing list.
I don't think the redundancy really hurts anyone. In particular because
this often happens for getters, where no one expects interesting
documentation in the first place.
Even if were to remove something:
- Removing the description would make the online Javadocs a less
readable because it is used in the method summary.
- Removing the @return tag is problematic because it would be difficult
(read: impossible) to enforce anything if there were optional.
- The Javadocs structure becomes intentionally inconsistent which to me
is even worse from a professionalism point-of-view.
All in all I wouldn't change anything.
On 15/10/2021 12:03, Jing Ge wrote:
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.png
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
>
