ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vyacheslav Daradur <daradu...@gmail.com>
Subject Re: method arguments code style
Date Tue, 08 May 2018 18:47:41 GMT
Hi, Igniters!

After the completion of publishing abbr-plugin [1][2] we will be able
to automate checking of method arguments code style.

It will be easy to check rules approved by the community during writing code.

[1] https://issues.apache.org/jira/browse/IGNITE-5698
[2] http://apache-ignite-developers.2346864.n4.nabble.com/abbrevation-rules-plugin-td19356.html

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



-- 
Best Regards, Vyacheslav D.

Mime
View raw message