Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: AmazonOrderItemList::setOrderId fails, though I'm giving it a string #175

Open
mehgcap opened this issue Jan 30, 2019 · 2 comments

Comments

@mehgcap
Copy link

mehgcap commented Jan 30, 2019

I'm pretty new to working with Amazon, so I hope this isn't a very stupid question. I have an order ID, and I'm trying to get a list of items so I can then get a list of available services. Some of my code is below, slightly modified as the actual code is spread across multiple files and classes:

$amazonOrderNumber = "123-8765432-1234567";
try {
	//first, we need the list of items and quantities
	$logger->debug("Setting up the object to get item details for order {$amazonOrderNumber}.");
	$itemsListObj = new \AmazonOrderItemList();
	if(!$itemsListObj->setOrderId($amazonOrderNumber))
		throw new \Exception("Invalid order number used when getting the list of items. Order number: {$amazonOrderNumber}. Order number variable type: " . gettype($amazonOrderNumber) . ".");
	$logger->debug("Fetching the items/quantities for order $amazonOrderNumber.");

Every time I run this, the exception is thrown. My order number is a string, as the debug log statement shows in my log, but setOrderId won't accept it for some reason. The function seems only to care that the order ID be a string or a number, so I can't imagine why I am getting stuck at this stage. What is the very obvious, simple thing I'm doing wrong? :) Thanks in advance for any ideas!

@mehgcap
Copy link
Author

mehgcap commented Jan 30, 2019

As is so often the case, I figured out the problem soon after posting my question. However, I would now like to make a suggestion.

The problem was that I used the "if(!$obj->function())" syntax. As the functions I was testing return false on failure, but nothing on success, PHP was interpreting the null response from the functions as false. If I instead use "if($obj->function() === false)", things work perfectly.

This leads me to my suggestion. If a function can return a boolean, and returns false on failure, it should return true on success. I'm not a PHP expert, so there may well be a good reason this library is set up this way. Please ignore me if this is the case. It just seems like it would make more sense to return a boolean value no matter what, rather than not returning anything on success. Thanks.

@Peardian
Copy link
Collaborator

You're right, returning true in the setter functions would make the design much better. Thank you for the suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants