Skip to content

refactor(message-parser): optimize code point matchers#20

Merged
ramsey merged 2 commits intomainfrom
refactor/optimize-code-point-matchers
Dec 13, 2021
Merged

refactor(message-parser): optimize code point matchers#20
ramsey merged 2 commits intomainfrom
refactor/optimize-code-point-matchers

Conversation

@ramsey
Copy link
Copy Markdown
Contributor

@ramsey ramsey commented Dec 11, 2021

Description

I had a long-ish thread on Twitter, where I discussed the code for these matchers with some friends, and I ran some benchmarks to evaluate various versions of the code. I ended up optimizing the code by collapsing the ranges in these logical expressions so that they are more concise, and this ended up significantly improving these functions.

I also added docblocks and unit tests.

Product requirements and context

The matchers were slow, and I wondered whether I could optimize them.

How has this been tested?

PR Checklist

  • I have added tests to cover my changes.

@ramsey ramsey requested review from a team, jmauerhan and xiian December 11, 2021 00:06
Comment thread src/Icu/MessageFormat/Parser/Util/IsPatternSyntax.php
Comment thread src/Icu/MessageFormat/Parser/Util/IsPatternSyntax.php
Comment thread src/Icu/MessageFormat/Parser/Util/IsWhiteSpace.php
@ramsey ramsey force-pushed the refactor/optimize-code-point-matchers branch from 86a4db6 to a927da0 Compare December 11, 2021 23:34
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 762d488 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3

The test coverage on the diff in this pull request is 100.0% (80% is the threshold).

This pull request will bring the total coverage in the repository to 96.0% (0.0% change).

View more on Code Climate.

@ramsey ramsey merged commit 66ee59e into main Dec 13, 2021
@ramsey ramsey deleted the refactor/optimize-code-point-matchers branch December 13, 2021 14:39
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 this pull request may close these issues.

2 participants