Convert ISBN10 to ISBN13 and change add to add_if_new #205
Conversation
Codecov Report
@@ Coverage Diff @@
## development #205 +/- ##
=================================================
+ Coverage 65.72% 71.99% +6.26%
- Complexity 1201 1212 +11
=================================================
Files 8 8
Lines 2343 2278 -65
=================================================
+ Hits 1540 1640 +100
+ Misses 803 638 -165
Continue to review full report at Codecov.
|
|
Unless there are circumstances when one might want to add an ISBN without undergoing this check, my suggestion would be to call the function once from within add_if_new, rather than each time add_if_new is called. One fewer thing to remember when adding calls in the future, it catches cases where the parameter name is now hard-coded (e.g. conversion of ID parameter), and it keeps all the handy format checks in one place. |
|
I will do that |
|
This fails the quality control because add_if_new correctly sets the "year" parameter instead of the "date" parameter. |
|
This is now called in add_if_new |
GlazerMann
changed the title
|
I think this is done now. |
Template.php
Outdated
| public function isbn10Toisbn13 ($isbn10) { | ||
| $isbn10 = trim($isbn10); // Remove leading and trailing spaces | ||
| $isbn10 = str_replace(array('—','―','–','−','‒'),'-', $isbn10); // standardize dahses : en dash, horizontal bar, em dash, minus sign, figure dash, to hyphen. | ||
| if (preg_match("/[^0123456789Xx-]/",$isbn10) === 1) return $isbn10; // Contains invalid characters |
There was a problem hiding this comment.
Need to escape the hyphen.
"/[^0123456789Xx\-]/"
Or more concisely
"/[^0-9Xx\-]/"
For consistency I'd also suggest using ~ instead of / as the regular expression delimiter.
There was a problem hiding this comment.
Are you able to add a test case too, please?
No description provided.