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. |
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 <[hidden email] >: > 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. > |
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 <[hidden email]>: > 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 < > [hidden email] > >: > > > 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. > > > |
Alexey,
+1. I personally also follow this style. On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < [hidden email]> 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 <[hidden email]>: > > > 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 < > > [hidden email] > > >: > > > > > 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. > > > > > > |
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 < [hidden email]> wrote: > Alexey, > > +1. > > I personally also follow this style. > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > [hidden email]> 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 <[hidden email]>: > > > > > 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 < > > > [hidden email] > > > >: > > > > > > > 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. > > > > > > > > > > |
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 <[hidden email]> 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 < > [hidden email]> wrote: > > > Alexey, > > > > +1. > > > > I personally also follow this style. > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > > [hidden email]> 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 <[hidden email]>: > > > > > > > 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 < > > > > [hidden email] > > > > >: > > > > > > > > > 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. > > > > > > > > > > > > > > > |
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 < [hidden email]> 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 <[hidden email]> > 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 < > > [hidden email]> wrote: > > > > > Alexey, > > > > > > +1. > > > > > > I personally also follow this style. > > > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > > > [hidden email]> 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 <[hidden email]>: > > > > > > > > > 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 < > > > > > [hidden email] > > > > > >: > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > |
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 <[hidden email]> 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 < > [hidden email]> 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 <[hidden email]> > > 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 < > > > [hidden email]> wrote: > > > > > > > Alexey, > > > > +1. > > > > I personally also follow this style. > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > > > > [hidden email]> 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 <[hidden email]>: > > > > > > 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 < > > > > > > [hidden email] > > > > > > > : > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > |
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 <[hidden email] >: > 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 <[hidden email]> 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 < > > [hidden email]> 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 <[hidden email]> > > > 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 < > > > > [hidden email]> wrote: > > > > > > > > > Alexey, > > > > > +1. > > > > > I personally also follow this style. > > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > > > > > [hidden email]> 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 <[hidden email] > >: > > > > > > > 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 < > > > > > > > [hidden email] > > > > > > > > : > > > > > > > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
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 <[hidden email]>: > 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 < > [hidden email]>: > >> 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 <[hidden email]> 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 < >> > [hidden email]> 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 <[hidden email] >> > >> > > 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 < >> > > > [hidden email]> wrote: >> > > > >> > > > > Alexey, >> > > > > +1. >> > > > > I personally also follow this style. >> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < >> > > > > [hidden email]> 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 <[hidden email] >> >: >> > > > > > > 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 < >> > > > > > > [hidden email] >> > > > > > > > : >> > > > > > > >> > > > > > > >> > > > > > > > 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. >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >> >> |
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 <[hidden email]> 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 <[hidden email]>: > >> 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 < >> [hidden email]>: >> >>> 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 <[hidden email]> 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 < >>> > [hidden email]> 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 <[hidden email] >>> > >>> > > 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 < >>> > > > [hidden email]> wrote: >>> > > > >>> > > > > Alexey, >>> > > > > +1. >>> > > > > I personally also follow this style. >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < >>> > > > > [hidden email]> 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 <[hidden email] >>> >: >>> > > > > > > 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 < >>> > > > > > > [hidden email] >>> > > > > > > > : >>> > > > > > > >>> > > > > > > >>> > > > > > > > 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. |
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 <[hidden email]>: > 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 <[hidden email]> > 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 <[hidden email]>: > > > >> 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 < > >> [hidden email]>: > >> > >>> 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 <[hidden email]> > 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 < > >>> > [hidden email]> 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 < > [hidden email] > >>> > > >>> > > 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 < > >>> > > > [hidden email]> wrote: > >>> > > > > >>> > > > > Alexey, > >>> > > > > +1. > >>> > > > > I personally also follow this style. > >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > >>> > > > > [hidden email]> 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 < > [hidden email] > >>> >: > >>> > > > > > > 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 < > >>> > > > > > > [hidden email] > >>> > > > > > > > : > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > 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. > |
In reply to this post by daradurvs
Hi Vyacheslav,
plugin was published in AI wiki, feel free to create ticket to add method code style check. Actually I'm not sure it is easy to validate code style in plugin, but I guess you know it better. Sincerely, Dmitriy Pavlov вт, 8 мая 2018 г. в 21:47, Vyacheslav Daradur <[hidden email]>: > 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 <[hidden email]> > 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 <[hidden email]>: > > > >> 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 < > >> [hidden email]>: > >> > >>> 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 <[hidden email]> > 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 < > >>> > [hidden email]> 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 < > [hidden email] > >>> > > >>> > > 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 < > >>> > > > [hidden email]> wrote: > >>> > > > > >>> > > > > Alexey, > >>> > > > > +1. > >>> > > > > I personally also follow this style. > >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < > >>> > > > > [hidden email]> 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 < > [hidden email] > >>> >: > >>> > > > > > > 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 < > >>> > > > > > > [hidden email] > >>> > > > > > > > : > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > 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. > |
Hi, I filed the ticket https://issues.apache.org/jira/browse/IGNITE-8512
I'll do this when I have enough time if it won't be done earlier. On Mon, May 14, 2018 at 8:46 PM, Dmitry Pavlov <[hidden email]> wrote: > Hi Vyacheslav, > > plugin was published in AI wiki, feel free to create ticket to add method > code style check. > > Actually I'm not sure it is easy to validate code style in plugin, but I > guess you know it better. > > Sincerely, > Dmitriy Pavlov > > вт, 8 мая 2018 г. в 21:47, Vyacheslav Daradur <[hidden email]>: > >> 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 <[hidden email]> >> 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 <[hidden email]>: >> > >> >> 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 < >> >> [hidden email]>: >> >> >> >>> 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 <[hidden email]> >> 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 < >> >>> > [hidden email]> 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 < >> [hidden email] >> >>> > >> >>> > > 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 < >> >>> > > > [hidden email]> wrote: >> >>> > > > >> >>> > > > > Alexey, >> >>> > > > > +1. >> >>> > > > > I personally also follow this style. >> >>> > > > > On Thu, May 3, 2018 at 12:45 PM, Alexey Goncharuk < >> >>> > > > > [hidden email]> 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 < >> [hidden email] >> >>> >: >> >>> > > > > > > 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 < >> >>> > > > > > > [hidden email] >> >>> > > > > > > > : >> >>> > > > > > > >> >>> > > > > > > >> >>> > > > > > > > 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. >> -- Best Regards, Vyacheslav D. |
Free forum by Nabble | Edit this page |