On Thu, May 10, 2018 at 8:08 PM, Anthony Emengo <aeme...@pivotal.io> 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 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. > 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 nodes. > I’m not a fan of singletons myself, but I feel like we’re 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 > > > Sincerely, > > Anthony and Victoria >
