search

pixelead0 / tmdb_v3-PHP-API-

Wrapper in PHP for The Movie Data Base version 3
119 stars 82 forks source link

Added getCrew() and getCast() to Movie and TvShow #49

Closed bogdanfinn closed 7 years ago

bogdanfinn commented 7 years ago

For Issue #48

pixelead0 commented 7 years ago

@bogdanfinn Thank's

webstractions commented 7 years ago

Looks good from what I see.

You might rename that the class to ApiMediaObject. You already of the MEDIA_TYPE_X constants. It is either Tv or Movie.

bogdanfinn commented 7 years ago

@webstractions Yeah, according to the naming i was not sure. I thought about creating two classes, a ApiBaseObject and a ApiMediaObject extending the ApiBaseObject, cause Movie and TVShow have a lot of similar methods, but Episode, Season, Person, etc. have also a few of the "Basemethods" every API Object should have. Then Movie and TVShow could extends the ApiMediaObject and Season, Episode and so on could extend die ApiBaseObject...

Thank you for you feedback!

webstractions commented 7 years ago

@bogdanfinn Yep, having the ApiBaseObject is a good call. It would be a small class, but you can hook other things into it like loggers, json methods, etc that the others can inherit.

These are best left as abstract classes. Put them in their own folder like Abstract\MediaObject.php, Abstract\CreditsObject.php, Abstract\PersonObject.php, yada, yada. There are objects all over the place if you dig into it deep enough. Not sure how far you want to take that. Some are used in several places (like Person) and make sense to use.

bogdanfinn commented 7 years ago

@webstractions Thanks for the feedback. I'm going to refactor the code a litte bit more and make antother PR with your improvements.

Another Question: What is your opinion in refactor all "Getter Methods" to never access the $_data property directly. Instead of it the getter should use the public function get($key); method, because this method has additional checks to avoid "Undefined index"-errors. For example: I would change the method: public function getOverview() { return $this->_data['overview']; } to public function getOverview() { return $this->get('overview'); } And the get()-Method is placed in the ApiBaseObject ;-)

Second Question: What is your opinion about using namespaces and an autoloader according to PSR-4 ?