Page MenuHomePhabricator

Reduce CRAP in FenParser0x88
Open, MediumPublic

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

Related Objects

StatusSubtypeAssignedTask
OpenWugapodes
OpenNone
OpenBUG REPORTWugapodes

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.

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

DannyS712 updated the task description. (Show Details)Mar 11 2020, 11:26 AM
DannyS712 added a subscriber: DannyS712.

It helps to remove a bunch of the unused code

DannyS712 moved this task from Unsorted to In progress on the User-DannyS712 board.

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

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

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

Currently pending patches split out

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

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

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

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

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

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

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

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

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 added a subscriber: dmaza.Dec 26 2020, 1:53 PM