Re: [pgadmin4][patch] Initial patch to decouple from ACI Tree

On Mon, Apr 30, 2018 at 11:27 PM, Joao De Almeida Pereira <
jdealmeidapereira(at)pivotal(dot)io> wrote:

> You've lost us with the last reply. We'd love to know what we'd have to do
> to get this patch committed. You mention that "some things are missing at
> some places" and that "there are missing bits".
>
I've already mentioned few of them in the earlier mails about them.

> Please note that this is not a complete solution that we've offered so far.
>
I know and, hence - asked for your understanding of the code (and, asked
for the TODOs.)

> But only one step in a grander effort to effect a more cleaner, more
> maintainable, more testable, code base.
>
Yes - we all want cleaner, maintainable, and testable code base.
As per my understading comments are required for better understanding, and
makes the code more maintainable.

But - if we're going to change everything in one go, it is difficult to
understand the changes.
Let's avoid mix match multiple things in a single patch. And, concerntrate
one functionality at a time.

-- Thanks, Ashesh

>
> Thanks
> Joao and Anthony.
>
> On Mon, Apr 30, 2018 at 11:45 AM Ashesh Vashi <
> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>
>> On Mon, Apr 30, 2018 at 9:07 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>>
>>>
>>> On Mon, Apr 30, 2018 at 4:33 PM, Ashesh Vashi <
>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>> On Mon, Apr 30, 2018 at 8:58 PM, Anthony Emengo <aemengo(at)pivotal(dot)io>
>>>> wrote:
>>>>
>>>>> I was expecting a separate layer between the tree implementation, and
>>>>>> aciTree adaptor.
>>>>>> Please find the patch for the example.
>>>>>> It will separate the two layers, and easy to replace with the new
>>>>>> implementation in future.
>>>>>
>>>>>
>>>>> In general, we want defer the separation of the layers for now. Even
>>>>> though we might assume that this is the direction we want to go in. It's
>>>>> simply too early to be making such an architectural leap. For right now, we
>>>>> just know that we need the decoupling, but don't know what if we'd need the
>>>>> 2 layers *as implemented*. The principle we're adhering to here is
>>>>> the Last Responsible Moment principle, which states that you only make the
>>>>> changes that you feel is necessary for the given problems you're facing:
>>>>> https://blog.codinghorror.com/the-last-responsible-moment/
>>>>>
>>>>> I would not like to see that changes in this patch.
>>>>>> I would like us to come up with the actual design about the hot
>>>>>> pluggability before going in this direction.
>>>>>
>>>>>
>>>>> In our point of view these 2 changes are not related. One thing is the
>>>>> internal code organization of the application, other thing is allowing
>>>>> third party to drop code in the application and it just work. These 2
>>>>> should be talked separately, but the hot pluggability is not something that
>>>>> will be address by this work we are doing right now.
>>>>>
>>>> Neither - it should be part of this change.
>>>> It should be addressed separately, and discussed.
>>>>
>>>
>>> I agree. As long as this work doesn't make the pluggability problem
>>> worse, that problem should be considered separately.
>>>
>>> So given Anthony's comments, are you happy with this patch?
>>>
>> I liked the design so far.
>> But - as Khushboo mentioned ealier - it is missing at some places.
>> I had to read through the code to understand the execution flow for some.
>>
>> And, there is still a lot missing bits, and pieces to consider for commit
>> in the repo.
>>
>> -- Thanks, Ashesh
>>
>>>
>>>
>>>>
>>>> -- Thanks, Ashesh
>>>>
>>>>>
>>>>> Anthony && Joao
>>>>>
>>>>> On Mon, Apr 30, 2018 at 11:03 AM, Ashesh Vashi <
>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Apr 30, 2018 at 8:30 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> On Sat, Apr 28, 2018 at 3:55 AM, Joao De Almeida Pereira <
>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>
>>>>>>>> Hi Hackers,
>>>>>>>> As you are aware we kept on working on the patch, so we are
>>>>>>>> attaching to this email a new version of the patch.
>>>>>>>> This new version contains all the changes in the previous one plus
>>>>>>>> more extractions of functions and refactoring of code.
>>>>>>>>
>>>>>>>> The objective of this patch is to create a separation between
>>>>>>>> pgAdmin and the ACI Tree. We are doing this because we realized that at
>>>>>>>> this point in time we have the ACI Tree all over the code of pgAdmin. I
>>>>>>>> found a very interesting article that really talks about this:
>>>>>>>> https://medium.freecodecamp.org/code-dependencies-are-the-
>>>>>>>> devil-35ed28b556d
>>>>>>>>
>>>>>>>> In this patch there are some visions and ideas about the location
>>>>>>>> of the code, the way to organize it and also try to pave the future for a
>>>>>>>> application that is stable, easy to develop on and that can be release at a
>>>>>>>> times notice.
>>>>>>>>
>>>>>>>> We are investing a big chunk of our time in doing this refactoring,
>>>>>>>> but while doing that we also try to respond to the patches sent to the
>>>>>>>> mailing list. We would like the feedback from the community because we
>>>>>>>> believe this is a changing point for the application. The idea is to change
>>>>>>>> the way we develop this application, instead of only correcting a bug of
>>>>>>>> developing a feature, with every commit we should correct the bug or
>>>>>>>> develop a feature but leave the code a little better than we found it
>>>>>>>> (Refactoring, refactoring, refactoring). This is hard work but that is what
>>>>>>>> the users from pgAdmin expect from this community of developers.
>>>>>>>>
>>>>>>>>
>>>>>>>> ======
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It is a huge patch
>>>>>>>> 86 files changed, 5492 inserts, 1840
>>>>>>>> deletions
>>>>>>>> and we would like to get your feedback as soon as possible, because
>>>>>>>> we are continuing to work on it which means it is going to grow in size.
>>>>>>>>
>>>>>>>>
>>>>>>>> At this point in time we still have 124 of 176 calls to the
>>>>>>>> function itemData from ACITree.
>>>>>>>>
>>>>>>>> What does each patch contain:
>>>>>>>> 0001:
>>>>>>>> Very simple patch, we found out that the linter was not looking
>>>>>>>> into all the javascript test files, so this patch will ensure it is
>>>>>>>>
>>>>>>> Committed the patch along with the regression introduced because of
>>>>>>> this patch.
>>>>>>>
>>>>>>>>
>>>>>>>> 0002:
>>>>>>>> New Tree abstraction. This patch contains the new Tree that works
>>>>>>>> as an adaptor for ACI Tree and is going to be used on all the extractions
>>>>>>>> that we are doing.
>>>>>>>>
>>>>>>>
>>>>>>> I was expecting a separate layer between the tree implementation,
>>>>>>> and aciTree adaptor.
>>>>>>> Please find the patch for the example.
>>>>>>>
>>>>>>> It will separate the two layers, and easy to replace with the new
>>>>>>> implemenation in future.
>>>>>>>
>>>>>>
>>>>>> Oops forgot to attach the patch.
>>>>>> Please find the patch attached.
>>>>>>
>>>>>> -- Thanks, Ashesh
>>>>>>
>>>>>>>
>>>>>>>> 0003:
>>>>>>>> Code that extracts, wrap with tests and replace ACI Tree
>>>>>>>> invocations.
>>>>>>>>
>>>>>>> There are many small cases left in the patches.
>>>>>>> Hence - I would like to know the TODO list created by you.
>>>>>>>
>>>>>>> e.g. When we remove any of the object from the database server,
>>>>>>> we're not yet removing the respective node from the new implementation, and
>>>>>>> its children.
>>>>>>>
>>>>>>>>
>>>>>>>> We start creating new pattern for the location of Javascript
>>>>>>>> files and their structure.
>>>>>>>>
>>>>>>> I would not like to see that changes in this patch.
>>>>>>> I would like us to come up with the actual design about the hot
>>>>>>> pluggability before going in this direction.
>>>>>>>
>>>>>>>> Create patterns for creation of dialogs (backup and restore)
>>>>>>>>
>>>>>>> It's better - we don't change the directory structure at the moment.
>>>>>>>
>>>>>>> I am not against dividing the big javascript files in small chunks,
>>>>>>> but - I would like us to discuss first about the hot plugins design first.
>>>>>>>
>>>>>>> -- Thanks, Ashesh
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Joao
>>>>>>>>
>>>>>>>>
>>>>>>>> On Fri, Apr 27, 2018 at 5:34 AM Ashesh Vashi <
>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> I have quite a few comments for the patch.
>>>>>>>>> I will send them soon.
>>>>>>>>>
>>>>>>>>> On Fri, Apr 27, 2018, 14:45 Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>>>>>
>>>>>>>>>> How is your work on this going Ashesh? Will you be committing
>>>>>>>>>> today?
>>>>>>>>>>
>>>>>>>>>> On Wed, Apr 25, 2018 at 8:52 AM, Dave Page <dpage(at)pgadmin(dot)org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Ashesh; you had agreed to work on this early this week. Please
>>>>>>>>>>> ensure you do so today.
>>>>>>>>>>>
>>>>>>>>>>> Thanks.
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Apr 24, 2018 at 8:13 PM, Joao De Almeida Pereira <
>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>
>>>>>>>>>>>> Can someone review and merge this patch?
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>> Joao
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Apr 18, 2018 at 10:23 AM Joao De Almeida Pereira <
>>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>> Any other comment about this patch?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Apr 10, 2018 at 12:00 PM Joao De Almeida Pereira <
>>>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hello Khushboo
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, Apr 9, 2018 at 1:59 AM Khushboo Vashi <
>>>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Joao,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I have reviewed your patch and have some suggestions.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Sat, Apr 7, 2018 at 12:43 AM, Joao De Almeida Pereira <
>>>>>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hello Murtuza/Dave,
>>>>>>>>>>>>>>>> Yes now the extracted functions are spread into different
>>>>>>>>>>>>>>>> files. The intent would be to make the files as small as possible, and also
>>>>>>>>>>>>>>>> to group and name them in a way that would be easy to understand what each
>>>>>>>>>>>>>>>> file is doing without the need of opening it.
>>>>>>>>>>>>>>>> As a example:
>>>>>>>>>>>>>>>> static/js/backup will contain all the backup related
>>>>>>>>>>>>>>>> functionality inside of this folder we can see the file:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> menu_utils.js At this moment in time we decided to group
>>>>>>>>>>>>>>>> all the functions that are related to the menu, but we can split that also
>>>>>>>>>>>>>>>> if we believe it is easier to see.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It's really very good to see the separated code for backup
>>>>>>>>>>>>>>> module. As we have done for backup, we would like do it for other PG
>>>>>>>>>>>>>>> utilities like restore, maintenance etc.
>>>>>>>>>>>>>>> Considering this, we should separate the code in a way that
>>>>>>>>>>>>>>> some of the common functionalities can be used for other modules like menu
>>>>>>>>>>>>>>> (as you have mentioned above), dialogue factory etc.
>>>>>>>>>>>>>>> Also, I think these functionalities should be in their
>>>>>>>>>>>>>>> respective static folder instead of pgadmin/static.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> About the location of the files. The move of the files to
>>>>>>>>>>>>>> pgadmin/static/js was made on purpose in order to clearly separate
>>>>>>>>>>>>>> Javascript from python code.
>>>>>>>>>>>>>> The rational behind it was
>>>>>>>>>>>>>> - Create a clear separation between the backend and frontend
>>>>>>>>>>>>>> - Having Javascript code concentrated in a single place,
>>>>>>>>>>>>>> hopefully, will encourage to developers to look for a functionality, that
>>>>>>>>>>>>>> is already implemented in another modules, because they are right there.
>>>>>>>>>>>>>> (When we started this journey we realized that the 'nodes' have a big
>>>>>>>>>>>>>> groups of code that could be shared, but because the Javascript is spread
>>>>>>>>>>>>>> everywhere it is much harder to look for it)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There are some drawbacks of this separation:
>>>>>>>>>>>>>> - When creating a new module we will need to put the
>>>>>>>>>>>>>> javascript in a separate location from the backend code
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> static/js/datagrid folder contains all the datagrid
>>>>>>>>>>>>>>>> related functionality
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Same as backup module, this should be in it's respective
>>>>>>>>>>>>>>> static/js folder.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Inside of the folder we can see the files:
>>>>>>>>>>>>>>>> get_panel_title.js is responsible for retrieving the name
>>>>>>>>>>>>>>>> of the panel
>>>>>>>>>>>>>>>> show_data.js is responsible for showing the datagrid
>>>>>>>>>>>>>>>> show_query_tool.js is responsible for showing the query
>>>>>>>>>>>>>>>> tool
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Does this structure make sense?
>>>>>>>>>>>>>>>> Can you give an example of a comment that you think is
>>>>>>>>>>>>>>>> missing and that could help?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> As a personal note, unless the algorithm is very obscure or
>>>>>>>>>>>>>>>> very complicated, I believe that if the code needs comments it is a signal
>>>>>>>>>>>>>>>> that something needs to change in terms of naming, structure of the part in
>>>>>>>>>>>>>>>> question. This being said, I am open to add some comments that might help
>>>>>>>>>>>>>>>> people.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> You are right, with the help of naming convention and
>>>>>>>>>>>>>>> structure of the code, any one can get the idea about the code. But it is
>>>>>>>>>>>>>>> very useful to understand the code
>>>>>>>>>>>>>>> very easily with the proper comments especially when there
>>>>>>>>>>>>>>> are multiple developers working on a single project.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I found some of the places where it would be great to have
>>>>>>>>>>>>>>> comments.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> - treeMenu: new tree.Tree() in a browser.js
>>>>>>>>>>>>>>> - tree.js (especially Tree class)
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> About the comment point I need a more clear understanding on
>>>>>>>>>>>>>> what kind of comments you are looking for. Because when you read the
>>>>>>>>>>>>>> function names you understand the intent, what they are doing. The
>>>>>>>>>>>>>> parameters also explain what you need to pass into them.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> If what you are looking for in these comments is the
>>>>>>>>>>>>>> reasoning being the change itself, then that should be present in the
>>>>>>>>>>>>>> commit message. Specially because this is going to be a very big patch with
>>>>>>>>>>>>>> a very big number of changes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>>> ​
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Fri, Apr 6, 2018 at 4:48 AM Murtuza Zabuawala <
>>>>>>>>>>>>>>>> murtuza(dot)zabuawala(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Joao,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Patch looks good and working as expected.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I also agree with Dave, Can we please add some comments in
>>>>>>>>>>>>>>>>> each file which can help us to understand the flow, I'm saying because now
>>>>>>>>>>>>>>>>> the code is segregated in so many separate files it will be hard to keep
>>>>>>>>>>>>>>>>> track of the flow from one file to another when debugging.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>>>>>> Regards,
>>>>>>>>>>>>>>>>> Murtuza Zabuawala
>>>>>>>>>>>>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>>>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Thu, Apr 5, 2018 at 7:08 PM, Joao De Almeida Pereira <
>>>>>>>>>>>>>>>>> jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi Khushboo,
>>>>>>>>>>>>>>>>>> Attached you can find both patches rebased
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Thu, Apr 5, 2018 at 6:31 AM Khushboo Vashi <
>>>>>>>>>>>>>>>>>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Hi Joao,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Can you please rebase the second patch?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>> Khushboo
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Tue, Apr 3, 2018 at 12:15 AM, Joao De Almeida Pereira
>>>>>>>>>>>>>>>>>>> <jdealmeidapereira(at)pivotal(dot)io> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hi Hackers,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Attached you can find the patch that will start to
>>>>>>>>>>>>>>>>>>>> decouple pgAdmin from ACITree library.
>>>>>>>>>>>>>>>>>>>> This patch is intended to be merged after 3.0, because
>>>>>>>>>>>>>>>>>>>> we do not want to cause any entropy or delay the release, but we want to
>>>>>>>>>>>>>>>>>>>> start the discussion and show some code.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> This job that we started is a massive tech debt chore
>>>>>>>>>>>>>>>>>>>> that will take some time to finalize and we would love the help of the
>>>>>>>>>>>>>>>>>>>> community to do it.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> *Summary of the patch:*
>>>>>>>>>>>>>>>>>>>> 0001 patch:
>>>>>>>>>>>>>>>>>>>> - Creates a new tree that will allow us to create a
>>>>>>>>>>>>>>>>>>>> separation between the application and ACI Tree
>>>>>>>>>>>>>>>>>>>> - Creates a Fake Tree (Test double, for reference on
>>>>>>>>>>>>>>>>>>>> the available test doubles: https://martinfowler.
>>>>>>>>>>>>>>>>>>>> com/bliki/TestDouble.html) that can be used to inplace
>>>>>>>>>>>>>>>>>>>> to replace the ACITree and also encapsulate the new tree behavior on our
>>>>>>>>>>>>>>>>>>>> tests
>>>>>>>>>>>>>>>>>>>> - Adds tests for all the tree functionalities
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 0002 patch:
>>>>>>>>>>>>>>>>>>>> - Extracts, refactors, adds tests and remove
>>>>>>>>>>>>>>>>>>>> dependency from ACI Tree on:
>>>>>>>>>>>>>>>>>>>> - getTreeNodeHierarchy
>>>>>>>>>>>>>>>>>>>> - on backup.js: menu_enabled, menu_enabled_server,
>>>>>>>>>>>>>>>>>>>> start_backup_global_server, backup_objects
>>>>>>>>>>>>>>>>>>>> - on datagrid.js: show_data_grid, get_panel_title,
>>>>>>>>>>>>>>>>>>>> show_query_tool
>>>>>>>>>>>>>>>>>>>> - Start using sprintf-js as Underscore.String is
>>>>>>>>>>>>>>>>>>>> deprecating sprintf function
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> This patch represents only 10 calls to ACITree.itemData
>>>>>>>>>>>>>>>>>>>> out of 176 that are spread around our code
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> *In Depth look on the process behind the patch:*
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> We started writing this patch with the idea that we
>>>>>>>>>>>>>>>>>>>> need to decouple pgAdmin4 from ACITree, because ACITree is no longer
>>>>>>>>>>>>>>>>>>>> supported, the documentation is non existent and ACITree is no longer being
>>>>>>>>>>>>>>>>>>>> actively developed.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Our process:
>>>>>>>>>>>>>>>>>>>> 1. We "randomly" selected a function that is part of
>>>>>>>>>>>>>>>>>>>> the ACITree. From this point we decided to replace that function with our
>>>>>>>>>>>>>>>>>>>> own version. The function that we choose was "itemData".
>>>>>>>>>>>>>>>>>>>> The function gives us all the "data" that a specific
>>>>>>>>>>>>>>>>>>>> node of the tree find.
>>>>>>>>>>>>>>>>>>>> Given in order to replace the tree we would need to
>>>>>>>>>>>>>>>>>>>> have a function that would give us the same information. We had 2 options:
>>>>>>>>>>>>>>>>>>>> a) Create a tree with a function called itemData
>>>>>>>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>>>>>>>> - At first view this was the simpler solution
>>>>>>>>>>>>>>>>>>>> - Would keep the status quo
>>>>>>>>>>>>>>>>>>>> Cons:
>>>>>>>>>>>>>>>>>>>> - Not a OOP approach
>>>>>>>>>>>>>>>>>>>> - Not very flexible
>>>>>>>>>>>>>>>>>>>> b) Create a tree that would return a node given an ID
>>>>>>>>>>>>>>>>>>>> and then the node would be responsible for giving it's data.
>>>>>>>>>>>>>>>>>>>> Pros:
>>>>>>>>>>>>>>>>>>>> - OOP Approach
>>>>>>>>>>>>>>>>>>>> - More flexible and we do not need to bring the tree
>>>>>>>>>>>>>>>>>>>> around, just a node
>>>>>>>>>>>>>>>>>>>> Cons:
>>>>>>>>>>>>>>>>>>>> - Break the current status quo
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Given these 2 options we decided to go for a more OOP
>>>>>>>>>>>>>>>>>>>> approach creating a Tree and a TreeNode classes, that in the future will be
>>>>>>>>>>>>>>>>>>>> renamed to ACITreeWrapper and TreeNode.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 2. After we decided on the starting point we searched
>>>>>>>>>>>>>>>>>>>> for occurrences of the function "itemData" and we found out that there were
>>>>>>>>>>>>>>>>>>>> 303 occurrences of "itemData" in the code and roughly 176 calls to the
>>>>>>>>>>>>>>>>>>>> function itself (some of the hits were variable names).
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 3. We selected the first file on the search and found
>>>>>>>>>>>>>>>>>>>> the function that was responsible for calling the itemData function.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 4. Extracted the function to a separate file
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 5. Wrap this function with tests
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 6. Refactor the function to ES6, give more declarative
>>>>>>>>>>>>>>>>>>>> names to variables and break the functions into smaller chunks
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 7. When all the tests were passing we replaced ACITree
>>>>>>>>>>>>>>>>>>>> with our Tree
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 8. We ensured that all tests were passing
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 9. Remove function from the original file and use the
>>>>>>>>>>>>>>>>>>>> new function
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 10. Ensure everything still works
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> 11. Find the next function and execute from step 4
>>>>>>>>>>>>>>>>>>>> until all the functions are replaced, refactored and tested.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> As you can see by the process this is a pretty huge
>>>>>>>>>>>>>>>>>>>> undertake, because of the number of calls to the function. This is just the
>>>>>>>>>>>>>>>>>>>> first step on the direction of completely isolating the ACITree so that we
>>>>>>>>>>>>>>>>>>>> can solve the problem with a large number of elements on the tree.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> *What is on our radar that we need to address:*
>>>>>>>>>>>>>>>>>>>> - Finish the complete decoupling of the ACITree
>>>>>>>>>>>>>>>>>>>> - Performance of the current tree implementation
>>>>>>>>>>>>>>>>>>>> - Tweak the naming of the Tree class to explicitly
>>>>>>>>>>>>>>>>>>>> tell us this is to use only with ACITree.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Thanks
>>>>>>>>>>>>>>>>>>>> Joao
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Dave Page
>>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>>
>>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Dave Page
>>>>>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>>>>>> Twitter: @pgsnake
>>>>>>>>>>
>>>>>>>>>> EnterpriseDB UK: http://www.enterprisedb.com
>>>>>>>>>> The Enterprise PostgreSQL Company
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>