From dev-return-34254-archive-asf-public=cust-asf.ponee.io@ignite.apache.org Tue May 8 20:47:49 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 620FE18063B for ; Tue, 8 May 2018 20:47:48 +0200 (CEST) Received: (qmail 86414 invoked by uid 500); 8 May 2018 18:47:47 -0000 Mailing-List: contact dev-help@ignite.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ignite.apache.org Delivered-To: mailing list dev@ignite.apache.org Received: (qmail 86398 invoked by uid 99); 8 May 2018 18:47:46 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 08 May 2018 18:47:46 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id 3B2041807BC for ; Tue, 8 May 2018 18:47:46 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.211 X-Spam-Level: ** X-Spam-Status: No, score=2.211 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_REPLY=1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001, URI_HEX=1.313] autolearn=disabled Authentication-Results: spamd3-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id 9Lfl69ft-dxY for ; Tue, 8 May 2018 18:47:44 +0000 (UTC) Received: from mail-pf0-f195.google.com (mail-pf0-f195.google.com [209.85.192.195]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id E0B1C5F405 for ; Tue, 8 May 2018 18:47:43 +0000 (UTC) Received: by mail-pf0-f195.google.com with SMTP id f20so15587358pfn.0 for ; Tue, 08 May 2018 11:47:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-transfer-encoding; bh=lWvV6Xx+nZ/l2oZzcHEFNA0jtehGX81LVapzJJOk0zw=; b=SUWZ6CTrQ72/TczlzQBFgg+D6zXtsTBAetCwmez4denTXzo8/kOiwmbPZhu/hJzQnz VGmEMZftH7DNi8BLN+4gPrh5Bwid+69Rx+BRaCorfafTDHI/j46wvVPitKgiACtRSH2z 4rxp1B6nNwGvEgSrPa3Iy6wusKd6ptWh5OBqM5E17wObr1Hf4fFd1V2HKnY4XyEz4hIw /TnnTp7IIP0xipug4BUJLYb21yj4lRvcSuF0rJyY0CVM55okUClaCozak8q1ZsMlPEvb jWKgkC1V5s1eKQDFnzBgEWtS6fpUIwg91VSqlnxSa4KJNUaBGcJ236W0Q8QqHkZ/gO+j dk4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:content-transfer-encoding; bh=lWvV6Xx+nZ/l2oZzcHEFNA0jtehGX81LVapzJJOk0zw=; b=dVlyhscRZXX0skDqhDUzavhpS+nGk+FdQN46/5KWwMw0bO7/Dr09GVfhkeieAcU6yM 9FzwY8dpB5AdAnarJxRAoNpJAXgNLwRlW/G2Igq0HiSKYNdPbtUzHKZywTKbTxdpGCrP zhxUDjpGzl7sdjaEpoJkABsiJsFXqyBx/pprNUE32K8JE+oFfPyd85lda2RAc14Y2Hcx QoQjJSf6JvSPpR2m2Pe656nTy6CbT3OWIrpbBY3Gz8qz/qJ6bhZ1nveFrGV5/SLM2gRP OyeHngcwdU1fdHxjKl2SsDpa6ODwqRO6q1VG92Fs413MI/0ewzrJ2JofAgJL8U0xIhSG MUJw== X-Gm-Message-State: ALQs6tD1vtFFMEjlmgGJLiq0qrGsEmpsHOnWLMime2axLbVI8cL2Mtvw CohQFq3z3ybOaLiJ6wWU+5S1AD+SkmLvdCmKgYYnTg== X-Google-Smtp-Source: AB8JxZqoqSTk/mnTrimG1mxZZ0F/GD7iIrgxUw/wmYbevO7YRsodI3CnVfP2Ca4A0anx7RQHvNmtOC9GNVymFWChwPc= X-Received: by 2002:a65:4c4d:: with SMTP id l13-v6mr33902187pgr.46.1525805262616; Tue, 08 May 2018 11:47:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.153.84 with HTTP; Tue, 8 May 2018 11:47:41 -0700 (PDT) In-Reply-To: References: <1525796751.local-c0b3583f-9f64-v1.2.1-7e7447b6@getmailspring.com> From: Vyacheslav Daradur Date: Tue, 8 May 2018 21:47:41 +0300 Message-ID: Subject: Re: method arguments code style To: dev@ignite.apache.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 cod= e. [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 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#Codi= ngGuidelines-MethodArguments > > Please review changes, so we can consider it is final. > > Sincerely, > Dmitriy Pavlov > > =D0=B2=D1=82, 8 =D0=BC=D0=B0=D1=8F 2018 =D0=B3. =D0=B2 20:05, Dmitry Pavl= ov : > >> I thought that Vladimir will update. >> >> By the way, Denis M, I propose to grant access to the wiki to Dmitry G. >> WDYT? >> >> =D0=B2=D1=82, 8 =D0=BC=D0=B0=D1=8F 2018 =D0=B3. =D0=B2 19:28, Dmitriy Go= vorukhin < >> dmitriy.govorukhin@gmail.com>: >> >>> Dmitriy, >>> >>> =D0=A1ould you please update code style wiki page in accordance with th= e >>> results of the discussion? >>> On May 7 2018, at 11:00 am, Vladimir Ozerov wrot= e: >>> > >>> > 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 whe= n >>> 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[] filte= rs, >>> > > int filter, SortOrder sortOrder, >>> > > HashSet 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 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 >> > >>> > > wrote: >>> > > >>> > > > My opinion is that we should allow both styles and not enforce an= y >>> of >>> > > them. >>> > > > I hardly can say that this >>> > > > >>> > > > public double getCost( >>> > > > Session ses, >>> > > > int[] masks, >>> > > > TableFilter[] filters, >>> > > > int filter, >>> > > > SortOrder sortOrder, >>> > > > HashSet 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 cols) { >>> > > > return SpatialTreeIndex.getCostRangeIndex(masks, >>> > > > table.getRowCountApproximation(), columns) / 10; >>> > > > } >>> > > > >>> > > > >>> > > > But this >>> > > > public CacheLockCandidates doneRemote( >>> > > > GridCacheVersion ver, >>> > > > Collection pending, >>> > > > Collection committed, >>> > > > Collection rolledback >>> > > > ) >>> > > > >>> > > > >>> > > > looks better than this >>> > > > public CacheLockCandidates doneRemote(GridCacheVersion ver, >>> > > > Collection pending, >>> > > > Collection committed, >>> > > > Collection 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 claus= e) >>> fits >>> > > > > >>> > > > >>> > > > the >>> > > > > > line. >>> > > > > > >>> > > > > > Thoughts? >>> > > > > > 2018-05-03 12:09 GMT+03:00 Dmitry Pavlov >> >: >>> > > > > > > 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 >>> > > > > > > >>> > > > > > > =D1=87=D1=82, 3 =D0=BC=D0=B0=D1=8F 2018 =D0=B3. =D0=B2 11:3= 1, 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, t= his >>> > > > 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 caches, >>> > > > > > > > Map templates) >>> > > > > > > > ``` >>> > > > > > > > I suggest two options for writing arguments. >>> > > > > > > > >>> > > > > > > > If arguments are placed in a line before the page delimit= er. >>> > > > > > > > ```java >>> > > > > > > > public void foo(Object obj1, Object obj2, Object obj3 , .= .. >>> , >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > > Object >>> > > > > > > objN){ >>> > > > > > > > .... >>> > > > > > > > } >>> > > > > > > > ``` >>> > > > > > > > If the arguments are not placed in the line before the pa= ge >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > > delimiter. >>> > > > > > > > >>> > > > > > > > ```java >>> > > > > > > > public void foo( >>> > > > > > > > Object obj1, >>> > > > > > > > Object obj2, >>> > > > > > > > Object obj3, >>> > > > > > > > ... , >>> > > > > > > > Object objN >>> > > > > > > > ){ >>> > > > > > > > .... >>> > > > > > > > } >>> > > > > > > > ``` >>> > > > > > > > In my personal experience, the last example is much easie= r >>> to >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > > merge >>> > > > > if >>> > > > > > > > method arguments were changed. >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >>> > >>> >>> --=20 Best Regards, Vyacheslav D.