refactor: move TaskGroupingTreeStorage to TaskGroups (#1957)

* refactor: rename storage var

* refactor: declare root in the TaskGroupingTree

* refactor: setGroupsHeadings() accepts TaskGroupingTreeStorage as parameter

* refactor: rename generateAllPaths()

* test: rename tree to root

* refactor: extract generateTaskTreeStorage()

* refactor: addTaskGroups() accepts groupingTreeStorage

* refactor: extract const groupingTreeStorage

* refactor: remove groupingTreeStorage from TaskGroupingTreeNode

* refactor: modify comment

* refactor: inline buildGroupingTree()

* refactor: comment & jsdoc in TaskGroupingTree

* refactor: move TaskGroupingTreeStorage to TaskGroups

* refactor: fix totalTasksCount() jsdoc

* refactor: rename vars in generateNodePath()

* refactor: extract TaskGroupingTreeStorage to a separate file

* refactor: revert changes in GroupingTreeNode

* refactor: rollback some comments

* refactor: Re-extract buildGroupingTree(), and reinstate its comments.

---------

Co-authored-by: Clare Macrae <[email protected]>

Author: ilandikov

src/Query/GroupDisplayHeadingSelector.ts

@@ -1,6 +1,6 @@
 import { GroupDisplayHeading } from './GroupDisplayHeading';
-import type { TaskGroupingTreeStorage } from './TaskGroupingTree';
 import type { Grouper } from './Grouper';
+import type { TaskGroupingTreeStorage } from './TaskGroupingTreeStorage';
 
 /*
  * This file contains implementation details of Group.ts

src/Query/TaskGroupingTree.ts

@@ -1,15 +1,7 @@
 import type { Task } from '../Task';
 import type { Grouper } from './Grouper';
 import { GroupingTreeNode } from './GroupingTreeNode';
-
-/**
- * Storage used for the initial grouping together of tasks.
- *
- * The keys of the map are the names of the groups.
- * For example, one set of keys might be ['Folder Name/', 'File Name']
- * and the values would be all the matching Tasks from that file.
- */
-export class TaskGroupingTreeStorage extends Map<string[], Task[]> {}
+import type { TaskGroupingTreeStorage } from './TaskGroupingTreeStorage';
 
 /*
  * A tree of tasks where every level in the tree corresponds to a grouping property.
@@ -48,30 +40,26 @@ class TaskGroupingTreeNode extends GroupingTreeNode<Task> {}
  * Ideally, this code would be simplified and moved in to TaskGroups.
  */
 export class TaskGroupingTree {
-    public groups = new TaskGroupingTreeStorage();
+    private root: TaskGroupingTreeNode;
 
     /**
-     * Group a list of tasks, according to one or more task properties
+     * Group a list of tasks, according to one or more task properties.
      * @param groupers 0 or more Grouping values, one per 'group by' line
      * @param tasks The tasks that match the task block's Query
      */
     constructor(groupers: Grouper[], tasks: Task[]) {
-        const tree = this.buildGroupingTree(groupers, tasks);
-        this.groups = tree.generateAllPaths();
+        // The root of the tree contains all the tasks.
+        this.root = new TaskGroupingTreeNode(tasks);
+
+        this.buildGroupingTree(groupers);
     }
 
-    /**
-     * Returns a grouping tree that groups the passed @tasks by the passed @groupers.
-     */
-    private buildGroupingTree(groupers: Grouper[], tasks: Task[]): TaskGroupingTreeNode {
+    private buildGroupingTree(groupers: Grouper[]) {
         // The tree is build layer by layer, starting from the root.
         // At every level, we iterate on the nodes of that level to generate
         // the next one using the next grouping.
 
-        // The root of the tree contains all the tasks.
-        const root = new TaskGroupingTreeNode(tasks);
-
-        let currentTreeLevel = [root];
+        let currentTreeLevel = [this.root];
         for (const grouper of groupers) {
             const nextTreeLevel = [];
             for (const currentTreeNode of currentTreeLevel) {
@@ -92,7 +80,14 @@ export class TaskGroupingTree {
             }
             currentTreeLevel = nextTreeLevel;
         }
+    }
 
-        return root;
+    /** Generates an intermediate storage for the initial grouping together of tasks.
+     *
+     * @returns a map where the keys are the names of the groups
+     * and the values are the tasks.
+     */
+    public generateTaskTreeStorage(): TaskGroupingTreeStorage {
+        return this.root.generateAllPaths();
     }
 }

src/Query/TaskGroupingTreeStorage.ts

@@ -0,0 +1,14 @@
+import type { Task } from '../Task';
+
+/**
+ * Storage used for the initial grouping together of tasks.
+ *
+ * The keys of the map are the names of the groups.
+ * For example, one set of keys might be ['Folder Name/', 'File Name']
+ * and the values would be all the matching Tasks from that file.
+ *
+ * This is an implementation detail of the task-grouping code, and does not need to
+ * be understood in order to group tasks.
+ */
+
+export class TaskGroupingTreeStorage extends Map<string[], Task[]> {}

src/Query/TaskGroups.ts

@@ -3,6 +3,7 @@ import type { Grouper } from './Grouper';
 import { GroupDisplayHeadingSelector } from './GroupDisplayHeadingSelector';
 import { TaskGroupingTree } from './TaskGroupingTree';
 import { TaskGroup } from './TaskGroup';
+import type { TaskGroupingTreeStorage } from './TaskGroupingTreeStorage';
 
 /**
  * Store all the groups of tasks generated by any 'group by'
@@ -30,11 +31,13 @@ export class TaskGroups {
         this._groupers = groups;
 
         const taskGroupingTree = new TaskGroupingTree(groups, tasks);
-        this.addTaskGroups(taskGroupingTree);
+        const groupingTreeStorage = taskGroupingTree.generateTaskTreeStorage();
+
+        this.addTaskGroups(groupingTreeStorage);
 
         this.sortTaskGroups();
 
-        this.setGroupsHeadings(taskGroupingTree);
+        this.setGroupsHeadings(groupingTreeStorage);
     }
 
     /**
@@ -89,8 +92,8 @@ export class TaskGroups {
         return output;
     }
 
-    private addTaskGroups(taskGroupingTree: TaskGroupingTree) {
-        for (const [groups, tasks] of taskGroupingTree.groups) {
+    private addTaskGroups(groupingTreeStorage: TaskGroupingTreeStorage) {
+        for (const [groups, tasks] of groupingTreeStorage) {
             const taskGroup = new TaskGroup(groups, tasks);
             this.addTaskGroup(taskGroup);
         }
@@ -124,8 +127,8 @@ export class TaskGroups {
         this._groups.sort(compareFn);
     }
 
-    private setGroupsHeadings(taskGroupingTree: TaskGroupingTree) {
-        const displayHeadingSelector = new GroupDisplayHeadingSelector(taskGroupingTree.groups, this._groupers);
+    private setGroupsHeadings(groupingTreeStorage: TaskGroupingTreeStorage) {
+        const displayHeadingSelector = new GroupDisplayHeadingSelector(groupingTreeStorage, this._groupers);
         for (const group of this._groups) {
             group.setGroupHeadings(displayHeadingSelector.getHeadingsForTaskGroup(group.groups));
         }

tests/GroupingTreeNode.test.ts

@@ -15,24 +15,24 @@ describe('Grouping Tree', () => {
         //        D[1]                  E[3, 4]       F[4, 5, 6]
         //                                            /        \
         //                                          G[4]       H[5, 6]
-        const tree = new GroupingTreeNode<number>([1, 2, 3, 4, 5, 6]);
+        const root = new GroupingTreeNode<number>([1, 2, 3, 4, 5, 6]);
         const b = new GroupingTreeNode<number>([1, 5, 6]);
         const c = new GroupingTreeNode<number>([3, 4, 5, 6]);
         const d = new GroupingTreeNode<number>([1]);
         const e = new GroupingTreeNode<number>([3, 4]);
         const f = new GroupingTreeNode<number>([4, 5, 6]);
         const g = new GroupingTreeNode<number>([4]);
         const h = new GroupingTreeNode<number>([5, 6]);
-        tree.children.set('B', b);
-        tree.children.set('C', c);
+        root.children.set('B', b);
+        root.children.set('C', c);
         b.children.set('D', d);
         c.children.set('E', e);
         c.children.set('F', f);
         f.children.set('G', g);
         f.children.set('H', h);
 
         // Act
-        const allLeafs = tree.generateAllPaths();
+        const allLeafs = root.generateAllPaths();
 
         // Assert
         const expected = new Map<string[], number[]>();
@@ -45,10 +45,10 @@ describe('Grouping Tree', () => {
 
     it("generates correct map when the node doesn't have children", () => {
         // Arrange
-        const tree = new GroupingTreeNode<number>([1, 2, 3, 4, 5, 6]);
+        const root = new GroupingTreeNode<number>([1, 2, 3, 4, 5, 6]);
 
         // Act
-        const allLeafs = tree.generateAllPaths();
+        const allLeafs = root.generateAllPaths();
 
         // Assert
         const expected = new Map<string[], number[]>();