From: | Anthony Emengo <aemengo(at)pivotal(dot)io> |
---|---|
To: | Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com> |
Cc: | Joao De Almeida Pereira <jdealmeidapereira(at)pivotal(dot)io>, Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>, Murtuza Zabuawala <murtuza(dot)zabuawala(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>, Khushboo Vashi <khushboo(dot)vashi(at)enterprisedb(dot)com> |
Subject: | Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree |
Date: | 2018-05-10 14:38:31 |
Message-ID: | CAG8BBZOKQDKtyu1-2HVCP5HFy1rKFdPVrMTY0SPEhb8x7Q7PBA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
>
> 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 functions
(setInode, unload, etc). Thus this is the easiest way to introduce our
abstraction and keep the functionality as before - at least until we decide
that whether we want to switch out the library or not.
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 nodes.
I’m not a fan of singletons myself, but I feel like we’re simply keeping
the architecture the same in the instance.
Sincerely,
Anthony and Victoria
From | Date | Subject | |
---|---|---|---|
Next Message | Akshay Joshi | 2018-05-11 06:23:54 | Re: [pgAdmin4][Patch]: RM #3277 Runtime startup error handling is broken on Windows. |
Previous Message | Dave Page | 2018-05-10 14:08:00 | Re: [pgAdmin4][Patch]: RM #3277 Runtime startup error handling is broken on Windows. |