Error handler

[codecov[bot]]

Codecov Report

Merging #6 into master will not change coverage. The diff coverage is 100%.

@@           Coverage Diff           @@
##             master     #6   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity      152    194   +42     
=======================================
  Files            23     24    +1     
  Lines           389    453   +64     
=======================================
+ Hits            389    453   +64

Impacted Files	Coverage Δ	Complexity Δ
src/GoogleMapsClient.php	100% <ø> (ø)	6 <0> (-1)	:arrow_down:
src/Directions/DirectionsResponse.php	100% <100%> (ø)	13 <8> (+8)	:arrow_up:
src/Errors/ApiException.php	100% <100%> (ø)	22 <22> (?)
src/TimeZone/TimeZoneRequest.php	100% <100%> (ø)	7 <3> (+2)	:arrow_up:
src/TimeZone/TimeZoneResponse.php	100% <100%> (ø)	15 <8> (+6)	:arrow_up:
src/GoogleMapsResponse.php	100% <100%> (ø)	10 <5> (+5)	:arrow_up:
src/GoogleMapsRequest.php	100% <100%> (ø)	8 <2> (ø)	: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 f69e1e9...564fa32. Read the comment docs.

[Evoluty]

To me : error handling is necessary but : --> all getters nullable makes the library just unusable : to me 2 options : check the success and the error before the factory call or directly in the factory. What's your opinion about that ? --> for the nullable datetime : not really sure about that as in the API it is a required integer value and I'd like to stick to the base API specs as much as possible. The user still can ->getTimestamp on his date not sure that that's the library job --> ofc at least we need to fix the tests and the coverage :)

[juherr]

--> all getters nullable makes the library just unusable : to me 2 options : check the success and the error before the factory call or directly in the factory. What's your opinion about that ?

Right, it was already checked after the factory method

--> for the nullable datetime : not really sure about that as in the API it is a required integer value and I'd like to stick to the base API specs as much as possible. The user still can ->getTimestamp on his date not sure that that's the library job

I disagree: The google api is designed without having a specific language in mind. Here, you are building a lib with the php savor with the usage of its std lib. It's what is done in the java lib where they ignore some attributes https://github.com/googlemaps/google-maps-services-java/blob/master/src/main/java/com/google/maps/TimeZoneApi.java#L53-L54 We can/should do the same here.

--> ofc at least we need to fix the tests and the coverage :)

Done

[Evoluty]

As stated IRL 🎉 : to merge this PR:

• [x] The DateTime result is added to the response as a big plus (use specificity of the language to parse result) and the existings properties are kept to match the official API documentation and for whoever needs them
• [x] The ZeroResult (even if it's the same behavior for the Google APIs for each APIs) needs a different behavior between APIs: for the timezone : the user uses bad parameters
• [ ] Response property not nullable could be kept by checking errors in the factory or in the constructor of the response class
• [ ] Fix the code coverage (can help if necessary)

[juherr]

100% coverage won't be possible for the moment because I added all exceptions and not only the ones used in the implemented api

[juherr]

Well, I don't understand why the assertion is failing.

[Evoluty]

As we discussed: assertion was failing because assertsame was checking references on DateTimeZone. We merged a pull request on this branch to have a better coverage and that fixed some errors (nullable) and guidelines (eof). I think I still have work on some unit tests that are lacking even with a good coverage. I'll add an issue on this. Waiting for the CI to run and then will merge this PR and do a new tag release on packagist. Next time we will need to split into more tiny pull requests to ease reviews.

Thanks a lot for your work @juherr ! That helps a lot.