Skip to content

London | 25-SDC-Nov | Aida Eslamimoghadam | Sprint 2 | Use precomputing for better performance - #145

Open
aydaeslami wants to merge 1 commit into
CodeYourFuture:mainfrom
aydaeslami:Improve-code-with-precomputing
Open

London | 25-SDC-Nov | Aida Eslamimoghadam | Sprint 2 | Use precomputing for better performance#145
aydaeslami wants to merge 1 commit into
CodeYourFuture:mainfrom
aydaeslami:Improve-code-with-precomputing

Conversation

@aydaeslami

Copy link
Copy Markdown

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

Introduced precomputing to reduce repeated work and improve performance of common_prefix and count_letters functions.

@aydaeslami aydaeslami added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 1, 2026

@cjyuan cjyuan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good.

Can you use complexity to explain how the new implementation is better than the original implementation?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 2, 2026
@aydaeslami

Copy link
Copy Markdown
Author

Thank you, @cjyuan, for your help and support.

For " find_longest_common_prefix" :
The previous implementation compared the common prefix of every pair of strings, resulting in a time complexity of O(n² × m). The new implementation sorts the strings first and only compares adjacent strings, reducing the time complexity to O(n log n + n × m).

And

For " count_letters" :
The previous implementation searched the string for each uppercase letter, resulting in a time complexity of O(n²). The new implementation uses sets for faster lookups, reducing the time complexity to O(n).

@aydaeslami aydaeslami added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 7, 2026
@cjyuan

cjyuan commented Jul 7, 2026

Copy link
Copy Markdown

Sorting strings involve comparing strings character by character. If we take into account, $m$, the average length of the strings in the given array, then the time complexity of sorting would also affected by $m$.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 7, 2026
@aydaeslami

Copy link
Copy Markdown
Author

Thanks @cjyuan, Yes, you are right. i missed the cost of string comparison during sorting.
ِThe overal complexity is :O(n log n × m + n × m)
simplifies to : O(n log n × m)

This part is a bit complicated for me. Please let me know if I need to change anything.

Many thanks.

@aydaeslami aydaeslami added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 20, 2026
@cjyuan

cjyuan commented Jul 20, 2026

Copy link
Copy Markdown

All good. Nothing needs to be changed.

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants