yannickfunk commented 3 years ago

As i found the req function a bit clunky, I tried to refactor it and make it more readable

codecov-commenter commented 3 years ago

Codecov Report

Merging #81 (f6be798) into master (10a356b) will decrease coverage by 0.15%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage   95.29%   95.14%   -0.16%     
==========================================
  Files          11       11              
  Lines         319      309      -10     
==========================================
- Hits          304      294      -10     
  Misses         15       15

Impacted Files	Coverage Δ
genanki/model.py	`92.53% <100.00%> (-0.97%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 10a356b...f6be798. Read the comment docs.

kerrickstaley commented 3 years ago

To me, this new version is about as hard to read and understand as the old version. The code is shorter but it uses a complicated regex and some non-trivial list comprehensions. This is mainly because what the _req() method does is complicated and so it's inherently hard to make the function simpler. Also, what is simple to one person may seem complicated to another and vice versa.

I did see one opportunity to make the existing code a little shorter and get rid of the variable fvcopy. I pushed that change: https://github.com/kerrickstaley/genanki/commit/73d3cbf36a493e8897b3cc1c909e9f5e45ae5f13

Closing this PR.

kerrickstaley / genanki

Make req function more readable #81

Codecov Report