Page MenuHomePhabricator
Create Task
Maniphest T246736

Reduce CRAP in FenParser0x88
Open, MediumPublic
Actions

Description

FenParser0x88 has an incredibly high CRAP at 118,000! (was: 224,000) This should be on the order of 10e0.

The following functions are the worst offenders

  • getValidMovePathsForPiece() - CRAP: 6k
  • getFromAndToByNotation() - CRAP: 1.9k
  • getValidSquaresOnCheck() - CRAP: 0.7k (was: 1.0k)
  • getCaptureAndProtectiveMoves() - CRAP: 0.5k (was: 0.7 k)
  • getNotationForAMove() - CRAP: 0.7k

They can be fixed by writing tests and refactoring.

See https://doc.wikimedia.org/cover-extensions/ChessBrowser/index.html

Details

SubjectRepoBranchLines +/-
Testing: Tests for FenParser0x88 and related fixesmediawiki/extensions/ChessBrowsermaster+538 -17
Add a separate SquareRelations classmediawiki/extensions/ChessBrowsermaster+324 -274
Simplify FenParser0x88::getValidMovesAndResultmediawiki/extensions/ChessBrowsermaster+177 -197
Add a separate CastlingTracker classmediawiki/extensions/ChessBrowsermaster+221 -78
Add a separate NotationAnalyzer classmediawiki/extensions/ChessBrowsermaster+200 -109
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 OpenNoneT239529 Create test cases
 OpenNoneT246736 Reduce CRAP in FenParser0x88
 ResolvedBUG REPORTWugapodesT246856 FenParser0x88 test fails

Event Timeline

Wugapodes created this task.Mar 3 2020, 6:24 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 3 2020, 6:24 AM
Wugapodes renamed this task from Reduce CRAP to Reduce CRAP in FenParser0x88.Mar 3 2020, 6:25 AM
Wugapodes triaged this task as Medium priority.
Wugapodes added a parent task: T239529: Create test cases.
gerritbot added a comment.Mar 4 2020, 1:07 AM

Change 576491 had a related patch set uploaded (by Wugapodes; owner: Wugapodes):
[mediawiki/extensions/ChessBrowser@master] Testing: Tests for FenParser0x88

https://gerrit.wikimedia.org/r/576491

gerritbot added a project: Patch-For-Review.Mar 4 2020, 1:07 AM
DannyS712 updated the task description. (Show Details)Mar 11 2020, 11:26 AM
DannyS712 subscribed.

It helps to remove a bunch of the unused code

DannyS712 added a project: User-DannyS712.Mar 11 2020, 11:29 AM
DannyS712 moved this task from Unsorted to In progress on the User-DannyS712 board.
gerritbot added a comment.Mar 29 2020, 8:33 PM

Change 584230 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ChessBrowser@master] Add a separate NotationAnalyzer class

https://gerrit.wikimedia.org/r/584230

gerritbot added a comment.Mar 30 2020, 12:35 AM

Change 584273 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ChessBrowser@master] Add a separate CastleUtis class

https://gerrit.wikimedia.org/r/584273

gerritbot added a comment.Mar 30 2020, 6:50 AM

Change 584296 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ChessBrowser@master] Add a separate SquareRelations class

https://gerrit.wikimedia.org/r/584296

DannyS712 added a comment.Mar 30 2020, 6:52 AM

Currently pending patches split out

Along with a number of patches to remove or simplify existing methods

gerritbot added a comment.Mar 30 2020, 7:01 AM

Change 584383 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ChessBrowser@master] Simplify FenParser0x88::getValidMovesAndResult

https://gerrit.wikimedia.org/r/584383

DannyS712 added a comment.Mar 30 2020, 7:31 AM
In T246736#6009221, @gerritbot wrote:

Change 584383 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/ChessBrowser@master] Simplify FenParser0x88::getValidMovesAndResult

https://gerrit.wikimedia.org/r/584383

Moves the big switch into a separate function

gerritbot added a comment.Mar 30 2020, 10:42 PM

Change 584230 merged by jenkins-bot:
[mediawiki/extensions/ChessBrowser@master] Add a separate NotationAnalyzer class

https://gerrit.wikimedia.org/r/584230

gerritbot added a comment.Mar 31 2020, 12:03 AM

Change 584273 merged by jenkins-bot:
[mediawiki/extensions/ChessBrowser@master] Add a separate CastlingTracker class

https://gerrit.wikimedia.org/r/584273

DannyS712 updated the task description. (Show Details)Mar 31 2020, 12:10 AM
gerritbot added a comment.Mar 31 2020, 12:16 AM

Change 584296 merged by jenkins-bot:
[mediawiki/extensions/ChessBrowser@master] Add a separate SquareRelations class

https://gerrit.wikimedia.org/r/584296

gerritbot added a comment.Mar 31 2020, 12:19 AM

Change 584383 merged by jenkins-bot:
[mediawiki/extensions/ChessBrowser@master] Simplify FenParser0x88::getValidMovesAndResult

https://gerrit.wikimedia.org/r/584383

DannyS712 updated the task description. (Show Details)Mar 31 2020, 12:54 AM
DannyS712 updated the task description. (Show Details)Apr 10 2020, 1:43 AM
DannyS712 updated the task description. (Show Details)
dmaza subscribed.Dec 26 2020, 1:53 PM
gerritbot added a comment.Apr 30 2021, 12:41 AM

Change 576491 merged by jenkins-bot:

[mediawiki/extensions/ChessBrowser@master] Testing: Tests for FenParser0x88 and related fixes

https://gerrit.wikimedia.org/r/576491

Restricted Repository Identity mentioned this in rECHB572b78fb1017: Testing: Tests for FenParser0x88 and related fixes.Apr 30 2021, 12:41 AM
Wugapodes closed subtask T246856: FenParser0x88 test fails as Resolved.Sep 22 2021, 12:11 AM
TheDJ subscribed.Sep 28 2021, 9:41 AM

At 36561 now.

TheDJ removed a project: Patch-For-Review.Sep 28 2021, 9:48 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL