Bug in the 978-613 range

[baschny]

Hi,

great library! I encountered a PHP warning while trying to apply it to convert this EAN to ISBN-13: 9786130971311

The problem is that in the 978-613 there is missing an "array ()" around the 'Range' key/value array. Are you generating this Ranges manually or automatically from some database?

[baschny]

there were also other ranges affected, I think you probably created them automatically, so you might want to check that script that did that... meanwhile the #4 fixes it but the diff is not very readable due to that huge array () line :)

[clemlatz]

@baschny Thanks for the fix but as you guessed, ranges are generated automatically from an XML file, so your changes would be overwritten the next time ranges are updated. The problem is in the dumpArraysFromFile method of the Ranges.php file if you want to try and fix it, otherwise I'll do it as soon as possible.

[baschny]

Ah, now I see. The problem is the simplexml_load_file method, which does a very "simple" conversion from XML to arrays. Without a DTD / Schema file it cannot know before-hand if a single XML sub-tag should be considered a "array with just one element" or a "singular item". Which is why it converts them differently in both cases.

So two potential solutions: Fix the array in PHP code before doing the var_export or handle this edge case (only one "Range") in the PHP code that reads the array. Will think about it.

[baschny]

Ok @iwazaru I fixed the problem in the generation code and added the automatism to create this PHP array with just one command line. It creates the ranges-array.php (which is also checked in for convenience for others) from the original XML sources. I removed your script which was probably just a starting point. Try it out, the interface to "Isbn" hasn't changed, the unit tests all pass - also my new one that proves that the Mauritius range works for example.

[clemlatz]

Wow, great job! Thanks! You guessed what I started to do and did far better than I would have.

However, I feel that the code used to update ranges should not be located in the Ranges class as it is not part of the API, but is more of a dev tool. As I see it, it won't be used by people who install the library via composer but only by people who clone the repository and update its code before submitting a pull request. Without it, the Ranges class is kind of useless so I think I'll just merge it with the Isbn class. I hope you're okay with that.

Thanks again!

[baschny]

👍 great stuff, thanks!