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 17:04:54 GMT
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.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> >
>
>

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