nkl-kst / the-sports-db

PHP library to get sports data from TheSportsDB (https://www.thesportsdb.com)
MIT License
23 stars 3 forks source link

More nullables on Teams #16

Closed shutupflanders closed 3 years ago

shutupflanders commented 3 years ago

Hey @nkl-kst Thanks for the 1.0.0 release! I'm now able to pull a good 90% of team information into my application, however I get the odd error that "x cannot be null":

# https://thesportsdb.com/api/v1/json/1/searchteams.php?t=kent
JSON property \"strManager\" in class \"NklKst\\TheSportsDb\\Entity\\Team\" must not be NULL
# https://thesportsdb.com/api/v1/json/1/searchteams.php?t=sussex
JSON property \"strAlternate\" in class \"NklKst\\TheSportsDb\\Entity\\Team\" must not be NULL

Perhaps it's better to allow any property in the Team Class to be nullable? I'm not sure how else to get around it, I'm sure the user can accept that not 100% of the properties will have a value due to the nature of the database.

Happy to open up a PR for the above if required.

nkl-kst commented 3 years ago

Hi Martin, thanks for testing release 1.0.0. I'd like to be as strict as possible for the entity properties, but I understand the problem here. Also I think this is not only a problem for the Team class, but for other entities too.

nkl-kst commented 3 years ago

I'd like to stay on a semi-strict way with the possibility to turn off the exceptions on non-nullable but missing fields. Making this feature from the underlying library configurable should work: https://github.com/cweiske/jsonmapper#nullables One could use the library with exceptions turned off if there are some fields missing, until the failing model is fixed here.

shutupflanders commented 3 years ago

Wouldn't that create the same issue? it's essentially background magic then if your validation passes or doesn't. Might be worth actually finding out which fields can be nullable from thesportsdb and mirroring it in this project.

nkl-kst commented 3 years ago

From the cweiske/jsonmapper docs:

JsonMapper throws an exception when a JSON property is null, unless the PHP class property has a nullable type - e.g. Contact|null.

If your API contains many fields that may be null and you do not want to make all your type definitions nullable, set: $jm->bStrictNullTypes = false;

This doesn't seem to work with typed properties. I decided to look through all entity attributes and made some of them nullable. Also I tested your two API requests with these changes, they should work now.

Please let me know if there are more fields that need a nullable fix, thanks in advance.

shutupflanders commented 3 years ago

Thanks @nkl-kst That appears to have fixed the initial problems with v1.0.0, I'll let you know if there are any others that appear over the next couple of weeks of our testing.