ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dmitriy Govorukhin <dmitriy.govoruk...@gmail.com>
Subject Re: method arguments code style
Date Sun, 06 May 2018 21:35:36 GMT
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