Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reducing time complexity #1668

Closed
5 tasks done
Vsinghal339-source opened this issue Sep 10, 2024 · 5 comments · Fixed by #1670
Closed
5 tasks done

Reducing time complexity #1668

Vsinghal339-source opened this issue Sep 10, 2024 · 5 comments · Fixed by #1670

Comments

@Vsinghal339-source
Copy link
Contributor

Vsinghal339-source commented Sep 10, 2024

Describe the bug

We found that the due to the below code, the time complexity is O(n square) which can be reduced to O(n)

Modified code:

 DataView.getAllSelectedFilteredItems = function () {
      /* istanbul ignore if */
      if (!Array.isArray(this.selectedRowIds)) {
        return [];
      }

      const selectedRowIdSet = new Set(this.selectedRowIds); // Add new set 
      const intersection = this.filteredItems.filter((a) => selectedRowIdSet.has(a[this.idProperty]));
      return (intersection || []);
    }

Reproduction

For large data set, the page hange for 1 minute

Which Framework are you using?

React

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | VERSION |
| Slickgrid-Universal | VERSION |
| TypeScript          | VERSION |
| Browser(s)          | VERSION |
| System OS           | VERSION |

Validations

@Vsinghal339-source
Copy link
Contributor Author

@ghiscoding Please do check this

@ghiscoding
Copy link
Owner

ghiscoding commented Sep 10, 2024

That's a good suggestion, and I would really like if you could contribute directly to the project and be added as a contributor to the project. Would you like to contribute? If you need help on how to contribute then let me know.

The code you are referring to can be found in the SlickDataView file, you could also contribute directly from GitHub since this seems like a small change

getAllSelectedFilteredItems<T extends TData>(): T[] {
/* v8 ignore next 3 */
if (!Array.isArray(this.selectedRowIds)) {
return [];
}
const intersection = this.filteredItems.filter((a) => this.selectedRowIds!.some((b) => a[this.idProperty as keyof TData] === b));
return (intersection || []) as T[];
}

Also doing a search across the project, I see another section where we could also do the same approach.

// collection of strings, just return the filtered string that are equals
if (this.collection.every(x => typeof x === 'number' || typeof x === 'string')) {
return this.collection.filter((c: SelectOption) => selectedValues?.some(val => `${val}` === c?.toString()));
}
// collection of label/value pair
const separatorBetweenLabels = this.collectionOptions?.separatorBetweenTextLabels ?? '';
const isIncludingPrefixSuffix = this.collectionOptions?.includePrefixSuffixToSelectedValues ?? false;
return this.collection
.filter(c => selectedValues.some(val => `${val}` === c?.[this.valueName]?.toString()))
.map(c => {

@Vsinghal339-source
Copy link
Contributor Author

@ghiscoding Yes please add me as a contributor. THanks

@ghiscoding
Copy link
Owner

ghiscoding commented Sep 10, 2024

@ghiscoding Yes please add me as a contributor. THanks

You need to contribute a Pull Request (PR) to be a contributor, so I will wait for your PR then 😉

@Vsinghal339-source
Copy link
Contributor Author

@ghiscoding Please review the PR #1670

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants