tboothman
Budget doesn't work reliably
I created a Pull Request, this method is more reliable although it is very hard to get the right data as it all has the same class name.
I added however a few thoughts about this method in the PR
@tboothman I don't understand why you asked for help while you not even bother to response to the issue or my PR?
TLDR:
- Sorry for never responding to this issue
- a budget_currency() method that returned //productionBudget/budget/currency might be a nice addition
- Issue resolved because budget() isn't flakey
Sorry, I stopped keeping track of these a while ago. Been in maintenance only for this project for ages because I don't really use it beyond getting the title,year,genres and rating from for some films. I'm not entirely sure what the realiability I was referring to was, and from reading https://github.com/tboothman/imdbphp/pull/246 I can't really tell what you two were aiming for.
Looking at the previous commit to the current one gives a better explanation of how this method worked (although it's odd to only want the budget if it's in $). https://github.com/tboothman/imdbphp/commit/02c233b053835942761be11c43261c81691706b0#diff-2a4bb245c110c1fe94bfeb5c0ae91d4435d947b30473ad5acabd1366a77b1776L3066 Return the amount of it's in USD, return null otherwise.
Inherited oddities are hard to deal with. There's quite a bit in this library that doesn't do what you might expect, or what it describes, or what you might want. Clearly a complete budget method would return the budget and the currency. Also, currently, the budget is returned even if it isn't in USD ... so the description doesn't match the functionality.
Now that I'm looking at it again the nextjson stuff is likely the graphql query that the page was built from. This is the budget data from the graphql query:
"productionBudget": {
"budget": {
"amount": 200000,
"currency": "NZD"
}
}
budget() is just returning the amount field.
There's only two paths for making budget() better imo. Make it do what the description says which is only return for USD OR add currency in as a new method. New method is better. It does state budget is usd only but nobody's noticed for 2 years .. so always returning a budget is the new bahaviour.
Again, apologies @duck7000. If you have any interest in making the change i'll get it merged as long as it's one of those options (or a reasonable other option)
@tboothman Apologies accepted but it might be better if you made it clear that you are in "maintenance" mode.. That would have explained why you didn't replied at all. But alright what's done is done, i don't have hard feelings now but i certainly did back then..
But back to budget As i recall the data from nextjson was/is not always the same, that is the issue as i remember
No i have personally no interest in this method as i'm not using it in my own version, so i'm not working on it, but i will change thoughts about it I do agree that budget should return budget and currency, as a number only doesn't say anything. so either return the whole string (budget and currency) and let it to the user to do with it what he/she wants. Or like you suggest return array with budget and currency. But currency is a issue i discussed with @jreklund like nz$ is not a real currency? I don't know what exactly the output is from the graphcl api but the currency maybe need some adjustments/fixes? As you stated above the output is the correct currency? so it is not a problem anymore i guess
I'm glad that you did implant the grapicl api as most users will apreciate it. Personally i will keep using my own version, the only thing sofar that is not working is Also known as, i can live with that.
I'm not sure new zealanders would appreciate you saying their currency isn't a real currency, lol.
I'm not interested in implementing it either, but the method would just be a straight copy paste of budget replacing the word budget with currency.