duck7000 / imdbGraphQLPHP

7 stars 0 forks source link

Suggestion - years less than 1000 #71

Closed GeorgeFive closed 1 month ago

GeorgeFive commented 1 month ago

Definitely a niche case, but I ran into it today, so I'll throw it out there. Some people have born / died years of less than 1000, ie

https://www.imdb.com/name/nm0678112/bio/

This class returns 66, which is correct, but most databases will not accept anything less than 1000 for a year if using a date type (which I assume everyone is), so this will be converted to 2066. However, 0066 is valid.

I fixed this on my end, but I didn't know if you had interest in adding something like this to the class for others?

        //support for years less than 1000
        if (preg_match_all("/[0-9]/", $born['year']) == '1') {
            $born['year'] = "000".$born['year']."";
        } elseif (preg_match_all("/[0-9]/", $born['year']) == '2') {
            $born['year'] = "00".$born['year']."";
        }

        //support for years less than 1000
        if (preg_match_all("/[0-9]/", $died['year']) == '1') {
            $died['year'] = "000".$died['year']."";
        } elseif (preg_match_all("/[0-9]/", $died['year']) == '2') {
            $died['year'] = "00".$died['year']."";
        }
duck7000 commented 1 month ago

Mm maybe we can use str_pad() to add zero's until four digits, the same is used for imdbid to add zero to match 8 digits.

I'll check in to this

duck7000 commented 1 month ago

Mm this is a interesting issue as MySQL for example only can use this in a data field

DATE

A date. The supported range is '1000-01-01' to '9999-12-31'.

So it might be advisable not to use DATE for this and use int or varchar fields

Your solution will obviously also work, but if you use DATE technically this isn't right as it is outside the support range.

I'm not sure how to "fix" this

GeorgeFive commented 1 month ago

I did see that, but I also saw that it's undocumented that the date type will handle, ie, 0066 properly. I saw that and tried it, and it does indeed work!

duck7000 commented 1 month ago

This is also true. I won't change the behavior of born() and died() as the output is correct. How anyone saves the data is up to them and is beyond the scope of this library.

But thanks for sharing this!