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 1fH1bH-0007Fu-Mi for pgadmin-hackers@arkaria.postgresql.org; Fri, 11 May 2018 06:32:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fH1bG-0008GI-9K for pgadmin-hackers@arkaria.postgresql.org; Fri, 11 May 2018 06:32:18 +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 1fH1bF-0008G8-SD for pgadmin-hackers@lists.postgresql.org; Fri, 11 May 2018 06:32:18 +0000 Received: from mail-ot0-x229.google.com ([2607:f8b0:4003:c0f::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fH1bD-00066q-3K for pgadmin-hackers@postgresql.org; Fri, 11 May 2018 06:32:16 +0000 Received: by mail-ot0-x229.google.com with SMTP id l12-v6so5063228oth.6 for ; Thu, 10 May 2018 23:32:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Mavmp9FbSbTVbhtDu2uYASdqHQHWL2hKOiyrEnqTREs=; b=hgzTPKpAV9Q5EQ0JE31JYa5hWwaovxEj3yB6a12Zisv70bpmC68iPuPjXTBdl/TB3Z 3V9YxgWedOtt7a2hANA+hh4VGrnZBQYo36Zjw5reUMeKai9o8tph01v25TN8jFsXXvXJ QAtbK+8h5AIRPCB6uoL5hOB3jj0Tfnt25N/9iUrksf4UT2Qy2piFfwj+hQ0b6a4r9vGz jUl4zVL8jM+FWPrq9sD+P+qYwZRUEYft8JPzKDlKZ8+ZyltDm6zcRokQkmTfh2DCIIKV LADUMPXZ6lBO8UVhytmDLfmXqZKaYsaMdykVrzoAtp5yCKI0a9PAprgMYwV7kfmYfLif aHVQ== 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:cc; bh=Mavmp9FbSbTVbhtDu2uYASdqHQHWL2hKOiyrEnqTREs=; b=UYKbkddWav+2fHNdELEhReBB45G1NFIxwLvF3ifYQJunLeqWMbQdaJ+zLRa8sUWYAS ju1ZioPvL+oe24v/4uUE5R0dyxnkCUFHgS726ZNbdwiuIU5tHQddfDNZbu6AGLMieF5A UcqD408OQJnaxn7jpEpulzp0Qhgm1KeAwxPFUN5WFO69+tUgkQLAVDi4FHG1uz7zJJYT YjL9WLNG9FZj5vGzsXUrVpRsHcdoNd8BxhE7TkEOkDZ2VdLy7v7H6LVXXI3ZpieInS70 z5B34CTTsJ5fwrvjdnkgmGgntDzJAR9hgOYnO+wSW97ZNvEPOskqeDnCU69w0iH40gNH O1WA== X-Gm-Message-State: ALKqPwcKncJWs+4QVCEZatoH5KtuWXtIp4irLJP33pltUyKdWJV2x7R9 urJVhcor/TeLPnkhiQfSnpQdkWSDn7xwEDNK3iOzIQ== X-Google-Smtp-Source: AB8JxZoOQPSG7pf6n9ataUQbWcguh6S05pNf0d14Z/9dMHuj8iI8HCKOwXgXjeRV3AycomZmlKsygdshOnI7bsOW/5o= X-Received: by 2002:a9d:3f6:: with SMTP id f109-v6mr2838149otf.228.1526020334052; Thu, 10 May 2018 23:32:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.138.5.130 with HTTP; Thu, 10 May 2018 23:31:53 -0700 (PDT) In-Reply-To: References: From: Ashesh Vashi Date: Fri, 11 May 2018 12:01:53 +0530 Message-ID: Subject: Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree To: Anthony Emengo Cc: Joao De Almeida Pereira , Akshay Joshi , Dave Page , Murtuza Zabuawala , pgadmin-hackers , Khushboo Vashi Content-Type: multipart/alternative; boundary="000000000000ee79e5056be84b3c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --000000000000ee79e5056be84b3c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, May 10, 2018 at 8:08 PM, Anthony Emengo wrote: > 1. In TreeNode, we're keepging the reference of DOMElement, do we really >> need it? > > As of right now, our Tree abstraction acts as an adapter to the aciTree > library. The aciTree library needs the domElement for most of its functio= ns > (setInode, unload, etc). Thus this is the easiest way to introduce our > abstraction and keep the functionality as before - at least until we deci= de > that whether we want to switch out the library or not. > I understand that. But - I've not seen any reference of domElement the code yet, hence - pointed that out. > 2. Are you expecting the tree class to be a singleton class > > Since this tree is referenced throughout the codebase, considering it to > be a singleton seems like the most appropriate pattern for this usecase. = It > is very much the same way how we create a single instance of the aciTree > library and use that throughout the codebase. Moreover, it opens up > opportunities to improve performance, for example caching lockups of node= s. > I=E2=80=99m not a fan of singletons myself, but I feel like we=E2=80=99re= simply keeping > the architecture the same in the instance. > Yeah - I don't see any usage of tree object from anywhere. And, we're already creating new object in browser.js (and, not utitlizing that instance anywhere.) -- Thanks, Ashesh > =E2=80=8B > > Sincerely, > > Anthony and Victoria > --000000000000ee79e5056be84b3c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
<= div style=3D"margin:0pt 0pt 8px">On Thu, May 10, 2018 at 8:08 PM, Anthony E= mengo <aemengo@pivotal.io> wrote:

1. In TreeNode, we&#= 39;re keepging the reference of DOMElement, do we really need it?

As of right now, our Tree abstraction acts as an adapter to the aciTree lib= rary. The aciTree library needs the domElement for most of its functions (s= etInode, unload, etc). Thus this is the easiest way to introduce our abstra= ction and keep the functionality as before - at least until we decide that = whether we want to switch out the library or not.

I understand that. But - I've not seen any reference of domElem= ent the code yet, hence - pointed that out.=C2=A0

2.=C2=A0Are you expecting the tree class to= be a singleton class

Since this tree is referenced t= hroughout the codebase, considering it to be a singleton seems like the mos= t appropriate pattern for this usecase. It is very much the same way how we= create a single instance of the aciTree library and use that throughout th= e codebase. Moreover, it opens up opportunities to improve performance, for= example caching lockups of nodes. I=E2=80=99m not a fan of singletons myse= lf, but I feel like we=E2=80=99re simply keeping the architecture the same = in the instance.

Yeah - I don't see an= y usage of tree object from anywhere.
And, we're already crea= ting new object in browser.js (and, not utitlizing that instance anywhere.)=

-- Thanks, Ashesh
=E2=80=8B

Sincerely,

Anthony and Victoria

--000000000000ee79e5056be84b3c--