Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fIyJt-0003Sr-1Y for pgadmin-hackers@arkaria.postgresql.org; Wed, 16 May 2018 15:26:25 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fIyJr-0001gl-M6 for pgadmin-hackers@arkaria.postgresql.org; Wed, 16 May 2018 15:26:23 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fIyJr-0001gb-8l for pgadmin-hackers@lists.postgresql.org; Wed, 16 May 2018 15:26:23 +0000 Received: from mx0a-00296801.pphosted.com ([148.163.150.38]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fIyJn-0003ON-VE for pgadmin-hackers@postgresql.org; Wed, 16 May 2018 15:26:22 +0000 Received: from pps.filterd (m0114581.ppops.net [127.0.0.1]) by mx0a-00296801.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w4GFGUgA026913 for ; Wed, 16 May 2018 15:26:18 GMT Received: from mail-oi0-f72.google.com (mail-oi0-f72.google.com [209.85.218.72]) by mx0a-00296801.pphosted.com with ESMTP id 2j0fx0ratn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT) for ; Wed, 16 May 2018 15:26:18 +0000 Received: by mail-oi0-f72.google.com with SMTP id x134-v6so743213oif.19 for ; Wed, 16 May 2018 08:26:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/lTDkThQcilkFP9uFNLqKRhbxesUuENv6hCBUNxFQ5A=; b=tPwrqLdrepA55PQ4rxsCHhgY4ASRU2HCXk7gneVhpKKKap/wnksj4ZqiRev3NVNYCm jZ26lbyDF/Z77HUZsitiU22HgqR4MPtHSCuxMOWlqdwO/mf4nIFqBt7feADiqE9gx4lY XHOb24ol7GUO7zEFdYhCJfb50gSzMzpuyLX8qDiaUQ7EGSsXXKEQbhjoA7nEKd0CYlrp uz88bo3dAhWLtx2nac0IPiMjHroM+6sewtpQA7uVojTrizZoGnWQMVmzVBmhBVTAS3aa dF7Ait9zk8/ceniy11HhQdXE1WAfpuwcrA7hGhR7ISr8SUEuTurBDw06KBBN3uZIAyqw yodA== X-Gm-Message-State: ALKqPwe//Zqd1F01iklxx/G97cEdUd68kyNgsQfhAuOF/3TFoPwM+2dr MQteU2y0pmnmv7kd9fOtDiPJt3e3Xzbg7OWHCWxx+YdwVjMCzrPdDagSKLC7qxIfc64+osAiCUH obqmd6b0QDRjUMMy0BhOJblW+5SO3X2fU0h5Vhmd6fOiDeF3FGyByrydBFqpUsctOaY9V X-Received: by 2002:aca:3445:: with SMTP id b66-v6mr816951oia.289.1526484376979; Wed, 16 May 2018 08:26:16 -0700 (PDT) X-Google-Smtp-Source: AB8JxZrQaSSrBP5bFeNVVJ6BPp5N8FDjVHv7X/8PjUBsj7iiSqFEJWS4kTlJV/E5Zer82QdgxVWrGqxEEWwM9dJ/4E8= X-Received: by 2002:aca:3445:: with SMTP id b66-v6mr816933oia.289.1526484376434; Wed, 16 May 2018 08:26:16 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Anthony Emengo Date: Wed, 16 May 2018 11:25:40 -0400 Message-ID: Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree To: Ashesh Vashi Cc: Joao De Almeida Pereira , Dave Page , Akshay Joshi , Murtuza Zabuawala , pgadmin-hackers , Khushboo Vashi Content-Type: multipart/alternative; boundary="0000000000000355b3056c545706" X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-05-16_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=1 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=360 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1711220000 definitions=main-1805160154 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --0000000000000355b3056c545706 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable export function canCreate(pgBrowser, childOfCatalogType) { return canCreateObject.bind({ browser: pgBrowser, childOfCatalogType: childOfCatalogType, }); } With respect to the above code: this bind pattern looks good and seems like the idiomatic way to handle this in JavaScript. On a lighter node, I don=E2= =80=99t even see the need for an additional method to wrap it. The invocation could have easily been like canCreate: canCreateObject.bind({ browser: pgBrowser, childOfCatalogType: childOfCatalogType }), I don=E2=80=99t feel too strongl= y here. I renamed it as isValidTreeNodeData, because - we were using it in for testing the tree data. Please suggest me the right place, and name. We=E2=80=99re not sure; maybe after continued refactoring, we will come acr= oss more generic functions. At that point we can revisit this and create a utils.js file. The original patch was separating them in different places, but - still uses some of the functionalities directly from the tree, which was happening because we have contextual menu. To give a better solution, I can think of putting the menus related code understand =E2=80=98sources/tree/menu=E2=80=99 directory. We=E2=80=99re particularly worried because we=E2=80=99re trying to avoid th= e coupling that we see in the code base today. We want to decouple *application state* from *business domain* logic as much as we can - because this makes the code much easier to understand. We achieve lower coupling by have more suitable interfaces to retrieve *application state* like: anyParent (the menu doesn=E2=80=99t c= are how this happens). This is the direction that we=E2=80=99re trying to move towa= rds, we just don=E2=80=99t want the package structure to undermine that developer i= ntent. How about nodeMenu.isSupportedNode(=E2=80=A6)? Naming is one of the hardest problems in programming. I don=E2=80=99t feel = too strongly about this one. For now, let=E2=80=99s keep it as is Thanks Anthony && Victoria =E2=80=8B --0000000000000355b3056c545706 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 
export function canCreate(pgBrowser=
, childOfCatalogType) {
  return canCreateObject.bind({
    browser: pgBrowser,
    childOfCatalogType: childOfCatalogType,
  });
}

With respect to th= e above code: this bind pattern looks good and seems like the idiomatic way= to handle this in JavaScript. On a lighter node, I don=E2=80=99t even see = the need for an additional method to wrap it. The invocation could have eas= ily been like canCreate: canCreateObject.bind({ browser: p= gBrowser, childOfCatalogType: childOfCatalogType }), I don=E2=80=99t= feel too strongly here.

I renamed it as isValidTreeNode= Data, because - we were using it in for testing the tree data. Please sugge= st me the right place, and name.

We=E2=80=99re not sure; maybe a= fter continued refactoring, we will come across more generic functions. At= that point we can revisit this and create a utils.js file.

The original patch was separati= ng them in different places, but - still uses some of the functionalities d= irectly from the tree, which was happening because we have contextual menu.=
To give a better solution, I can think of putting the menus related cod= e understand =E2=80=98sources/tree/menu=E2=80=99 directory.

We=E2=80=99re particularly worr= ied because we=E2=80=99re trying to avoid the coupling that we see in the c= ode base today. We want to decouple application state from bus= iness domain logic as much as we can - because this makes the code muc= h easier to understand. We achieve lower coupling by have more suitable int= erfaces to retrieve application state like: anyPa= rent (the menu doesn=E2=80=99t care how this happens). This is the d= irection that we=E2=80=99re trying to move towards, we just don=E2=80=99t w= ant the package structure to undermine that developer intent.

How about nodeMenu.isSupportedN= ode(=E2=80=A6)?

Naming is one of the hardest pr= oblems in programming. I don=E2=80=99t feel too strongly about this one. Fo= r now, let=E2=80=99s keep it as is

Thanks
Anthony && Vi= ctoria

=E2=80=8B



--0000000000000355b3056c545706--