ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrey Kuznetsov <stku...@gmail.com>
Subject Re: Coding guidelines. Useless JavaDoc comments.
Date Wed, 07 Aug 2019 10:54:33 GMT
+1 for making javadoc comments optional.

- Empty and tautological comments are kind of garbage that reduce
readability.
- It's better to leave the entity undocumented, than write
unexpressive/misleading comment.
- Even classes may not require javadocs, e.g. simple DTOs.

ср, 7 авг. 2019 г., 13:39 Denis Garus <garus.d.g@gmail.com>:

> Thx for feedback!
>
> >> we have to write proper javadoc for all production classes, including
> internal.
>
> Nikolay, I cannot agree with it.
>
> What should be the best comment for the next fields?
> /** */
> private static final long serialVersionUID = 0L;
> or
> /** */
> @LoggerResource
> private IgniteLogger log;
>
> There are more than 8000 lines of /** */ only at the ignite-core module (do
> not include tests).
>
> Any comments will be redundant and just noise. Obvious comment learns
> readers skip all comments.
>
>
> >> identical distance/padding/margin between fields in a class - is really
> cool
>
> Vyacheslav, but we have a blank line between fields. Why is one blank line
> not enough?
>
> ср, 7 авг. 2019 г. в 12:58, Павлухин Иван <vololo100@gmail.com>:
>
> > Hi,
> >
> > Denis, thank you for starting this discussion!
> >
> > My opinion here is that having a good javadoc for every class and
> > method is not feasible in the real world. I am quite curious to see a
> > non-trivial project which follows it. Also, all comments and javadocs
> > are prone to become misleading when code changes (human factor). In my
> > experience good method and variable names and clear code organization
> > often are more helpful than javadocs.
> >
> > ср, 7 авг. 2019 г. в 12:49, Vyacheslav Daradur <daradurvs@gmail.com>:
> > >
> > > I agree that useless comments look weird in the codebase.
> > >
> > > But, identical distance/padding/margin between fields in a class - is
> > > really cool, and helps read the class very fast.
> > >
> > > On Wed, Aug 7, 2019 at 12:26 PM Nikolay Izhikov <nizhikov@apache.org>
> > wrote:
> > > >
> > > > Hello, Denis.
> > > >
> > > > Thanks for starting this discussion.
> > > >
> > > > I think we have to write proper javadoc for all production classes,
> > including internal.
> > > > We should fix useless javadoc you provide.
> > > > We should not accept patches without good javadocs.
> > > >
> > > > As for the tests, seems, we can make javadoc optional.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > > В Ср, 07/08/2019 в 11:41 +0300, Denis Garus пишет:
> > > > > Igniters!
> > > > >
> > > > > I think it's time to change coding guidelines in part of JavaDoc
> > comments
> > > > > [1]:
> > > > > > > Every method, field or initializer public, private or protected
> > in
> > > > >
> > > > > top-level,
> > > > > > > inner or anonymous type should have at least minimal Javadoc
> > comments
> > > > >
> > > > > including
> > > > > > > description and description of parameters using @param,
@return
> > and
> > > > >
> > > > > @throws Javadoc tags,
> > > > > > > where applicable.
> > > > >
> > > > > Let's look at JavaDoc comments in the IgniteKernal class:
> > > > >
> > > > > Why?
> > > > >
> > > > > /** */ - 15 matches.
> > > > >
> > > > > What can you get new from these comments?
> > > > >
> > > > > /** Periodic starvation check interval. */
> > > > > private static final long PERIODIC_STARVATION_CHECK_FREQ = 1000 *
> 30;
> > > > >
> > > > > /** Long jvm pause detector. */
> > > > > private LongJVMPauseDetector longJVMPauseDetector;
> > > > >
> > > > > /** Scheduler. */
> > > > >     private IgniteScheduler scheduler;
> > > > >
> > > > > /** Stop guard. */
> > > > >     private final AtomicBoolean stopGuard = new AtomicBoolean();
> > > > >
> > > > > /**
> > > > >      * @param cfg Configuration to use.
> > > > >      * @param utilityCachePool Utility cache pool.
> > > > >      * @param execSvc Executor service.
> > > > >      * @param sysExecSvc System executor service.
> > > > >      * @param stripedExecSvc Striped executor.
> > > > >      * @param p2pExecSvc P2P executor service.
> > > > >      * @param mgmtExecSvc Management executor service.
> > > > >      * @param igfsExecSvc IGFS executor service.
> > > > >      * @param dataStreamExecSvc data stream executor service.
> > > > >      * @param restExecSvc Reset executor service.
> > > > >      * @param affExecSvc Affinity executor service.
> > > > >      * @param idxExecSvc Indexing executor service.
> > > > >      * @param callbackExecSvc Callback executor service.
> > > > >      * @param qryExecSvc Query executor service.
> > > > >      * @param schemaExecSvc Schema executor service.
> > > > >      * @param customExecSvcs Custom named executors.
> > > > >      * @param errHnd Error handler to use for notification about
> > startup
> > > > > problems.
> > > > >      * @param workerRegistry Worker registry.
> > > > >      * @param hnd Default uncaught exception handler used by thread
> > pools.
> > > > >      * @throws IgniteCheckedException Thrown in case of any errors.
> > > > >      */
> > > > >     public void start(
> > > > >         final IgniteConfiguration cfg,
> > > > >         ExecutorService utilityCachePool,
> > > > >         final ExecutorService execSvc,
> > > > >         final ExecutorService svcExecSvc,
> > > > >         final ExecutorService sysExecSvc,
> > > > >         final StripedExecutor stripedExecSvc,
> > > > >         ExecutorService p2pExecSvc,
> > > > >         ExecutorService mgmtExecSvc,
> > > > >         ExecutorService igfsExecSvc,
> > > > >         StripedExecutor dataStreamExecSvc,
> > > > >         ExecutorService restExecSvc,
> > > > >         ExecutorService affExecSvc,
> > > > >         @Nullable ExecutorService idxExecSvc,
> > > > >         IgniteStripedThreadPoolExecutor callbackExecSvc,
> > > > >         ExecutorService qryExecSvc,
> > > > >         ExecutorService schemaExecSvc,
> > > > >         @Nullable final Map<String, ? extends ExecutorService>
> > > > > customExecSvcs,
> > > > >         GridAbsClosure errHnd,
> > > > >         WorkersRegistry workerRegistry,
> > > > >         Thread.UncaughtExceptionHandler hnd,
> > > > >         TimeBag startTimer
> > > > >     )
> > > > >
> > > > > These comments look ugly and useless, and that is the main class
of
> > core.
> > > > > Why do they need us?
> > > > > Let us change the coding guidelines in part of JavaDoc comments:
> > > > > Every method public API should have at least minimal Javadoc
> > comments,
> > > > > including description and description of parameters using @param,
> > @return,
> > > > > and @throws Javadoc tags,
> > > > > where applicable.
> > > > > For internal API, JavaDoc comments should be optional. It's up to
a
> > > > > contributor or reviewer.
> > > > >
> > > > > What are you think?
> > > > >
> > > > > 1.
> > > > >
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-JavadocComments
> > >
> > >
> > >
> > > --
> > > Best Regards, Vyacheslav D.
> >
> >
> >
> > --
> > Best regards,
> > Ivan Pavlukhin
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message