ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitry Pavlov <dpavlov....@gmail.com>
Subject Re: method arguments code style
Date Tue, 08 May 2018 19:32:01 GMT
Hi Vyacheslav,

Can we proceed with plugin publishing? I have not seen any objections, so I
propose to complete this task.

Sincerely,
Dmitriy Pavlov

вт, 8 мая 2018 г. в 21:47, Vyacheslav Daradur <daradurvs@gmail.com>:

> Hi, Igniters!
>
> After the completion of publishing abbr-plugin [1][2] we will be able
> to automate checking of method arguments code style.
>
> It will be easy to check rules approved by the community during writing
> code.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-5698
> [2]
> http://apache-ignite-developers.2346864.n4.nabble.com/abbrevation-rules-plugin-td19356.html
>
> On Tue, May 8, 2018 at 8:34 PM, Dmitry Pavlov <dpavlov.spb@gmail.com>
> wrote:
> > Folks, I've messed with another topic, where Vladimir was going to update
> > review check-list.
> >
> > Here I've updated Coding Guidelines:
> >
> https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodArguments
> >
> > Please review changes, so we can consider it is final.
> >
> > Sincerely,
> > Dmitriy Pavlov
> >
> > вт, 8 мая 2018 г. в 20:05, Dmitry Pavlov <dpavlov.spb@gmail.com>:
> >
> >> I thought that Vladimir will update.
> >>
> >> By the way, Denis M, I propose to grant access to the wiki to Dmitry G.
> >> WDYT?
> >>
> >> вт, 8 мая 2018 г. в 19:28, Dmitriy Govorukhin <
> >> dmitriy.govorukhin@gmail.com>:
> >>
> >>> Dmitriy,
> >>>
> >>> Сould you please update code style wiki page in accordance with the
> >>> results of the discussion?
> >>> On May 7 2018, at 11:00 am, Vladimir Ozerov <vozerov@gridgain.com>
> wrote:
> >>> >
> >>> > Dmitry,
> >>> > Agree, mixed style when some arguments share the same line and others
> >>> don't
> >>> > looks very bad. My proposal was to allow two styles - first when all
> >>> > arguments are on the same line splitted by 120 char limit, second
> when
> >>> all
> >>> > every arguments is on a separate line.
> >>> > Mixed style should be disallowed.
> >>> >
> >>> > On Mon, May 7, 2018 at 12:35 AM, Dmitriy Govorukhin <
> >>> > dmitriy.govorukhin@gmail.com> wrote:
> >>> >
> >>> > > Vladimir,
> >>> > > My eyes cry when I see this
> >>> > > public double getCost(Session ses, int[] masks, TableFilter[]
> filters,
> >>> > > int filter, SortOrder sortOrder,
> >>> > > HashSet<Column> cols) {
> >>> > > return SpatialTreeIndex.getCostRangeIndex(masks,table.
> >>> > > getRowCountApproximation(),
> >>> > > columns) / 10;
> >>> > > }
> >>> > >
> >>> > > Why did arguments split into 3 line?
> >>> > > It is much better readable for me
> >>> > > public double getCost(
> >>> > > Session ses,
> >>> > > int[] masks,
> >>> > > TableFilter[] filters,
> >>> > > int filter,
> >>> > > SortOrder sortOrder,
> >>> > > HashSet<Column> cols
> >>> > > ) {
> >>> > > return SpatialTreeIndex.getCostRangeIndex(masks,
> >>> > > able.getRowCountApproximation(),
> >>> > > columns) / 10;
> >>> > > }
> >>> > >
> >>> > > Do we have a serious reason to continue writing code as before?
> >>> > >
> >>> > >
> >>> > > On Thu, May 3, 2018 at 2:24 PM, Vladimir Ozerov <
> vozerov@gridgain.com
> >>> >
> >>> > > wrote:
> >>> > >
> >>> > > > My opinion is that we should allow both styles and not enforce
> any
> >>> of
> >>> > > them.
> >>> > > > I hardly can say that this
> >>> > > >
> >>> > > > public double getCost(
> >>> > > > Session ses,
> >>> > > > int[] masks,
> >>> > > > TableFilter[] filters,
> >>> > > > int filter,
> >>> > > > SortOrder sortOrder,
> >>> > > > HashSet<Column> cols
> >>> > > > ) {
> >>> > > > return SpatialTreeIndex.getCostRangeIndex(masks,
> >>> > > > table.getRowCountApproximation(), columns) / 10;
> >>> > > > }
> >>> > > >
> >>> > > > is better than this
> >>> > > > public double getCost(Session ses, int[] masks, TableFilter[]
> >>> filters,
> >>> > > > int filter, SortOrder sortOrder,
> >>> > > > HashSet<Column> cols) {
> >>> > > > return SpatialTreeIndex.getCostRangeIndex(masks,
> >>> > > > table.getRowCountApproximation(), columns) / 10;
> >>> > > > }
> >>> > > >
> >>> > > >
> >>> > > > But this
> >>> > > > public CacheLockCandidates doneRemote(
> >>> > > > GridCacheVersion ver,
> >>> > > > Collection<GridCacheVersion> pending,
> >>> > > > Collection<GridCacheVersion> committed,
> >>> > > > Collection<GridCacheVersion> rolledback
> >>> > > > )
> >>> > > >
> >>> > > >
> >>> > > > looks better than this
> >>> > > > public CacheLockCandidates doneRemote(GridCacheVersion ver,
> >>> > > > Collection<GridCacheVersion> pending,
> >>> > > > Collection<GridCacheVersion> committed,
> >>> > > > Collection<GridCacheVersion> rolledback)
> >>> > > >
> >>> > > >
> >>> > > > The very problem is that our eyes feel comfortable when we
either
> >>> move
> >>> > > them
> >>> > > > horizontally, or vertically (example 2). But with proposed
code
> >>> style you
> >>> > > > have to do zigzag movements in general case because lines
are not
> >>> aligned
> >>> > > > (example 1).
> >>> > > > Merge conflicts on multiliners are hardly of major concern
for
> us.
> >>> > > >
> >>> > > > Vladimir.
> >>> > > > On Thu, May 3, 2018 at 1:46 PM, Eduard Shangareev <
> >>> > > > eduard.shangareev@gmail.com> wrote:
> >>> > > >
> >>> > > > > Alexey,
> >>> > > > > +1.
> >>> > > > > I personally also follow this style.
> >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk <
> >>> > > > > alexey.goncharuk@gmail.com> wrote:
> >>> > > > >
> >>> > > > > > Actually, I've been following the suggested code
style for
> >>> quite a
> >>> > > > while.
> >>> > > > > > I'm ok to add this to coding guidelines, however,
I think we
> >>> should
> >>> > > > >
> >>> > > >
> >>> > > > allow
> >>> > > > > > the old style when the method signature (without
throws
> clause)
> >>> fits
> >>> > > > >
> >>> > > >
> >>> > > > the
> >>> > > > > > line.
> >>> > > > > >
> >>> > > > > > Thoughts?
> >>> > > > > > 2018-05-03 12:09 GMT+03:00 Dmitry Pavlov <
> dpavlov.spb@gmail.com
> >>> >:
> >>> > > > > > > Hi Dmitriy,
> >>> > > > > > > I like your proposal, so +1 from me.
> >>> > > > > > > I think it would make code more readable and
easy to
> >>> understand.
> >>> > > > > > > Sincerely,
> >>> > > > > > > Dmitriy Pavlov
> >>> > > > > > >
> >>> > > > > > > чт, 3 мая 2018 г. в 11:31, Dmitriy
Govorukhin <
> >>> > > > > > > dmitriy.govorukhin@gmail.com
> >>> > > > > > > > :
> >>> > > > > > >
> >>> > > > > > >
> >>> > > > > > > > Hi folks,
> >>> > > > > > > > I read
> >>> > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > >
> >>> > > > Coding+Guidelines
> >>> > > > > ,
> >>> > > > > > > > but did not find anything about code
style for method
> >>> arguments.
> >>> > > > > > > >
> >>> > > > > > > > In many places in the code, I see different
code style,
> this
> >>> > > > creates
> >>> > > > > > > > difficulties for reading.
> >>> > > > > > > >
> >>> > > > > > > > It seems to me an example below is rather
difficult to
> >>> perceive.
> >>> > > > > > > > ```java
> >>> > > > > > > > public void foo(Object obj1,
> >>> > > > > > > > Object obj2, Object obj3,... ,Object
objN){
> >>> > > > > > > > ....
> >>> > > > > > > > }
> >>> > > > > > > > ```
> >>> > > > > > > > An example GridCacheProcessor.addCacheOnJoin(...)
> >>> > > > > > > >
> >>> > > > > > > > ```java
> >>> > > > > > > > private void addCacheOnJoin(CacheConfiguration<?,
?> cfg,
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > > > boolean
> >>> > > > > > > sql,
> >>> > > > > > > > Map<String, CacheInfo> caches,
> >>> > > > > > > > Map<String, CacheInfo> templates)
> >>> > > > > > > > ```
> >>> > > > > > > > I suggest two options for writing arguments.
> >>> > > > > > > >
> >>> > > > > > > > If arguments are placed in a line before
the page
> delimiter.
> >>> > > > > > > > ```java
> >>> > > > > > > > public void foo(Object obj1, Object obj2,
Object obj3 ,
> ...
> >>> ,
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > >
> >>> > > > Object
> >>> > > > > > > objN){
> >>> > > > > > > > ....
> >>> > > > > > > > }
> >>> > > > > > > > ```
> >>> > > > > > > > If the arguments are not placed in the
line before the
> page
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > > > delimiter.
> >>> > > > > > > >
> >>> > > > > > > > ```java
> >>> > > > > > > > public void foo(
> >>> > > > > > > > Object obj1,
> >>> > > > > > > > Object obj2,
> >>> > > > > > > > Object obj3,
> >>> > > > > > > > ... ,
> >>> > > > > > > > Object objN
> >>> > > > > > > > ){
> >>> > > > > > > > ....
> >>> > > > > > > > }
> >>> > > > > > > > ```
> >>> > > > > > > > In my personal experience, the last example
is much
> easier
> >>> to
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > >
> >>> > >
> >>> > > merge
> >>> > > > > if
> >>> > > > > > > > method arguments were changed.
> >>> > > > > > >
> >>> > > > > >
> >>> > > > >
> >>> > > >
> >>> > >
> >>> >
> >>> >
> >>>
> >>>
>
>
>
> --
> Best Regards, Vyacheslav D.
>

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