From 80be52fbb61c0f17010729a2c10f40ffe893bf74 Mon Sep 17 00:00:00 2001 From: Jin Wei Date: Sun, 6 Aug 2023 10:52:25 +0800 Subject: [PATCH 1/5] Improve efficiency of saving models --- src/app/core/services/issue.service.ts | 45 +++++++++++++++++--------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index 1ad1aed9f..217628be0 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -76,7 +76,7 @@ export class IssueService { getLatestIssue(id: number): Observable { return this.githubService.fetchIssueGraphql(id).pipe( map((response: GithubIssue) => { - this.createAndSaveIssueModel(response); + this.createAndSaveIssueModels([response]); return this.issues[id]; }), catchError((err) => { @@ -96,13 +96,21 @@ export class IssueService { /** * This function will update the issue's state of the application. This function needs to be called whenever a issue is added/updated. + * + * @params issuesToUpdate - An array of issues to update the state of the application with. + * @params shouldEmit - Whether the updated issues should be emitted to issues$. */ - updateLocalStore(issueToUpdate: Issue) { - this.issues = { - ...this.issues, - [issueToUpdate.id]: issueToUpdate - }; - this.issues$.next(Object.values(this.issues)); + updateLocalStore(issuesToUpdate: Issue[], shouldEmit: boolean = true) { + const newIssues = issuesToUpdate.reduce((obj, issue) => { + obj[issue.id] = issue; + return obj; + }, {}); + + this.issues = { ...this.issues, ...newIssues }; + + if (shouldEmit) { + this.issues$.next(Object.values(this.issues)); + } } reset(resetSessionId: boolean) { @@ -135,16 +143,15 @@ export class IssueService { return of([]); } - // const issuesAPICallsByFilter = filters.map(filter => this.githubService.fetchIssuesGraphql(filter)); return forkJoin(issuesAPICallsByFilter).pipe( - map((issuesByFilter: [][]) => { + map((issuesByFilter: GithubIssue[][]) => { const fetchedIssueIds: Array = []; // Take each issue and put it in next in issues$ - for (const issues of issuesByFilter) { + for (const githubIssues of issuesByFilter) { + const issues = this.createAndSaveIssueModels(githubIssues); for (const issue of issues) { - fetchedIssueIds.push(this.createIssueModel(issue).id); - this.createAndSaveIssueModel(issue); + fetchedIssueIds.push(issue.id); } } @@ -156,10 +163,16 @@ export class IssueService { ); } - private createAndSaveIssueModel(githubIssue: GithubIssue): boolean { - const issue = this.createIssueModel(githubIssue); - this.updateLocalStore(issue); - return true; + private createAndSaveIssueModels(githubIssues: GithubIssue[], shouldEmit: boolean = true): Issue[] { + const issues: Issue[] = []; + + for (const githubIssue of githubIssues) { + const issue = this.createIssueModel(githubIssue); + issues.push(issue); + } + this.updateLocalStore(issues, shouldEmit); + + return issues; } private deleteIssuesFromLocalStore(ids: Array): void { From 2531a1e6a2f97c0a7d85fac5824693cd60a84b39 Mon Sep 17 00:00:00 2001 From: Jin Wei Date: Sun, 6 Aug 2023 11:08:58 +0800 Subject: [PATCH 2/5] Improve efficiency of deleting outdated models --- src/app/core/services/issue.service.ts | 30 ++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index 217628be0..9eda4d3e8 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -85,15 +85,6 @@ export class IssueService { ); } - /** - * This function will update the issue's state of the application. This function needs to be called whenever a issue is deleted. - */ - deleteFromLocalStore(issueToDelete: Issue) { - const { [issueToDelete.id]: issueToRemove, ...withoutIssueToRemove } = this.issues; - this.issues = withoutIssueToRemove; - this.issues$.next(Object.values(this.issues)); - } - /** * This function will update the issue's state of the application. This function needs to be called whenever a issue is added/updated. * @@ -145,7 +136,7 @@ export class IssueService { return forkJoin(issuesAPICallsByFilter).pipe( map((issuesByFilter: GithubIssue[][]) => { - const fetchedIssueIds: Array = []; + const fetchedIssueIds: number[] = []; // Take each issue and put it in next in issues$ for (const githubIssues of issuesByFilter) { @@ -155,7 +146,7 @@ export class IssueService { } } - const outdatedIssueIds: Array = this.getOutdatedIssueIds(fetchedIssueIds); + const outdatedIssueIds: number[] = this.getOutdatedIssueIds(fetchedIssueIds); this.deleteIssuesFromLocalStore(outdatedIssueIds); return Object.values(this.issues); @@ -175,17 +166,24 @@ export class IssueService { return issues; } - private deleteIssuesFromLocalStore(ids: Array): void { - ids.forEach((id: number) => { - this.getIssue(id).subscribe((issue) => this.deleteFromLocalStore(issue)); - }); + private deleteIssuesFromLocalStore(ids: number[], shouldEmit: boolean = true): void { + const withoutIssuesToRemove = { ...this.issues }; + for (const id of ids) { + delete withoutIssuesToRemove[id]; + } + + this.issues = withoutIssuesToRemove; + + if (shouldEmit) { + this.issues$.next(Object.values(this.issues)); + } } /** * Returns an array of outdated issue ids by comparing the ids of the recently * fetched issues with the current issue ids in the local store */ - private getOutdatedIssueIds(fetchedIssueIds: Array): Array { + private getOutdatedIssueIds(fetchedIssueIds: number[]): number[] { /* Ignore for first fetch or ignore if there is no fetch result From 429d6e966e2be6eb7b34cd44b3eb7d583604986c Mon Sep 17 00:00:00 2001 From: Jin Wei Date: Mon, 7 Aug 2023 18:26:44 +0800 Subject: [PATCH 3/5] Improve readability and reduce one unnecessary copy in updateLocalStore --- src/app/core/services/issue.service.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index efa2593b5..48656f80d 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -92,12 +92,11 @@ export class IssueService { * @params shouldEmit - Whether the updated issues should be emitted to issues$. */ updateLocalStore(issuesToUpdate: Issue[], shouldEmit: boolean = true) { - const newIssues = issuesToUpdate.reduce((obj, issue) => { - obj[issue.id] = issue; - return obj; - }, {}); - - this.issues = { ...this.issues, ...newIssues }; + const newIssues = { ...this.issues }; + issuesToUpdate.forEach((issue) => { + newIssues[issue.id] = issue; + }); + this.issues = newIssues; if (shouldEmit) { this.issues$.next(Object.values(this.issues)); From a8ed9145799f8068b9d0cea035124719dd309deb Mon Sep 17 00:00:00 2001 From: Jin Wei Date: Wed, 13 Sep 2023 18:04:57 +0800 Subject: [PATCH 4/5] Make updateLocalStore private --- src/app/core/services/issue.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index 48656f80d..b5a737b17 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -91,7 +91,7 @@ export class IssueService { * @params issuesToUpdate - An array of issues to update the state of the application with. * @params shouldEmit - Whether the updated issues should be emitted to issues$. */ - updateLocalStore(issuesToUpdate: Issue[], shouldEmit: boolean = true) { + private updateLocalStore(issuesToUpdate: Issue[], shouldEmit: boolean = true) { const newIssues = { ...this.issues }; issuesToUpdate.forEach((issue) => { newIssues[issue.id] = issue; From 0a8765d8e78a601d4ddd2e14dba4c43756be60a7 Mon Sep 17 00:00:00 2001 From: Jin Wei Date: Wed, 13 Sep 2023 18:12:53 +0800 Subject: [PATCH 5/5] Remove shouldEmit flag --- src/app/core/services/issue.service.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/app/core/services/issue.service.ts b/src/app/core/services/issue.service.ts index b5a737b17..32b0f0885 100644 --- a/src/app/core/services/issue.service.ts +++ b/src/app/core/services/issue.service.ts @@ -89,18 +89,15 @@ export class IssueService { * This function will update the issue's state of the application. This function needs to be called whenever a issue is added/updated. * * @params issuesToUpdate - An array of issues to update the state of the application with. - * @params shouldEmit - Whether the updated issues should be emitted to issues$. */ - private updateLocalStore(issuesToUpdate: Issue[], shouldEmit: boolean = true) { + private updateLocalStore(issuesToUpdate: Issue[]) { const newIssues = { ...this.issues }; issuesToUpdate.forEach((issue) => { newIssues[issue.id] = issue; }); this.issues = newIssues; - if (shouldEmit) { - this.issues$.next(Object.values(this.issues)); - } + this.issues$.next(Object.values(this.issues)); } reset(resetSessionId: boolean) { @@ -151,19 +148,19 @@ export class IssueService { ); } - private createAndSaveIssueModels(githubIssues: GithubIssue[], shouldEmit: boolean = true): Issue[] { + private createAndSaveIssueModels(githubIssues: GithubIssue[]): Issue[] { const issues: Issue[] = []; for (const githubIssue of githubIssues) { const issue = this.createIssueModel(githubIssue); issues.push(issue); } - this.updateLocalStore(issues, shouldEmit); + this.updateLocalStore(issues); return issues; } - private deleteIssuesFromLocalStore(ids: number[], shouldEmit: boolean = true): void { + private deleteIssuesFromLocalStore(ids: number[]): void { const withoutIssuesToRemove = { ...this.issues }; for (const id of ids) { delete withoutIssuesToRemove[id]; @@ -171,9 +168,7 @@ export class IssueService { this.issues = withoutIssuesToRemove; - if (shouldEmit) { - this.issues$.next(Object.values(this.issues)); - } + this.issues$.next(Object.values(this.issues)); } /**