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.govoruk...@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 <voze...@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.shangar...@gmail.com> wrote:
> >
> > > Alexey,
> > >
> > > +1.
> > >
> > > I personally also follow this style.
> > >
> > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk <
> > > alexey.goncha...@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....@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.govoruk...@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.
> > > > > >
> > > > >
> > > >
> > >
> >
>
