tboothman / imdbphp

PHP library for retrieving film and tv information from IMDb
251 stars 84 forks source link

Working on episode method #234

Closed duck7000 closed 3 years ago

duck7000 commented 3 years ago

I'm working on the episodes method and hit a problem. This is part of the code that i came up with. It is not ready jet but this blocks any further progress.

            $xpath = $this->getXpathPage("Episodes");
            //$page = $this->getPage("Episodes");
            if (empty($xpath)) {
                return $this->season_episodes;
            } // no such page
            $sea = $xpath->query("//select[@id=\"bySeason\"]//option");
            foreach ($sea as $season) {
                $s = $season->getAttribute('value');
                $xpathSeasonPage = $this->getXpathPage("Episodes-$s");
                if (empty($xpathSeasonPage)) {
                    continue;
                } // no such page
            }

$xpathSeasonPage = $this->getXpathPage("Episodes-$s"); This line is not working, it works if i use getPage instead of getXpathPage.

I think the problem lies in this function getUrlSuffix($pageName)?

@tboothman can/will you look in to this? I don't want to use getPage because we want to get rid of regex.

tboothman commented 3 years ago

I would consider #214 if you're looking to change this method. There's more to be gained by fixing that problem than changing exactly how the parge is parsed.

getPage is a bit of a rats nest and I possibly caused some of it by trying to use abstractions to reuse a tiny little bit of code rather than just being explicit. It's not obvious to me why $this->getXpathPage("Episodes-5") would do anything other than call $this->getPage("Episodes-5") which does work regardless of how complicated its mechanism is. Are you sure that $s is a number and doesn't have any spaces around it or anything like that? The regex it's matching to know it's an season list page is quite strict '!^Episodes-(-?\d+)$!'

duck7000 commented 3 years ago

Well i don't know how far my skills will get me into fixing #214 but i will give it a try.

My first thought was to rewrite this method to xpath, then get everything working correctly. Other fixes that are needed after all this?

It is indeed possible that i made a mistake, hence it is not working for me, thanks for pointing me in a direction to get it working.

duck7000 commented 3 years ago

I thought about #214 In that case season select option contains a blank option (contains white space) so check if season contains that blank option and if so use select year?

A compare between season and year won't work as it is possible that the season count is lower or higher than year count.

What to do with unknown season or year remains the question

Thomasdouscha commented 3 years ago

Some TV Series are based years not seasons . If possible, it would be greatly appreciated if you consider this when coding.

duck7000 commented 3 years ago

@Thomasdouscha The problem is how to distinction seasons versus year based?

And there are numerous options possible, year based, season based, year unknown, season unknown, blank options in seasons, 1 season and multiple years or a combination of all of those

duck7000 commented 3 years ago

PR got merged, closing this