Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I am getting information from an XML document and for the most part everything runs as it should. Occasionally, I get a php fatal error Call to a member function children() on a non-object. I have tried to use this

if(isset($current->AttributeSets->children('ns2', true)->ItemAttributes->ListPrice->Amount))
{
    $listPrice = $current->AttributeSets->children('ns2', true)->ItemAttributes->ListPrice->Amount;
} else {
    $listPrice = 0;
}

to catch any times there is no information in $current, but it does not always work. How do I handle the errors so it works all the time?

share|improve this question
This should really be moved to SO as this is not a request for a review, but for help. Also, you might think about adding more information. There isn't nearly enough here to answer this question. A sample from the XML would be helpful, as would a full error stack. – mseancole Aug 28 '12 at 20:38
I think it depends what is being reviewed here. The error handling approach can be commented upon at least. – Paul Aug 29 '12 at 12:35
True, and for the OP to have gotten this deep into the XML tree I would have expected those pointers to have been abstracted more first. Instead it appears that he is navigating directly to them, which is causing some really unnecessary repetition and could cause mistakes/typos in the future. Try using variables to abstract the different sections of that XML while you are parsing it, such as $attributes = $current->AttributeSets; etc... – mseancole Aug 30 '12 at 13:06
The tree is pretty large, and $current holds all of the items that are static. Once I need to get custom variables, ie list price, then I just go to them directly ($current->AttributeSets->children('ns2', true)->ItemAttributes->ListPrice->Amount; – Jim Aug 30 '12 at 13:37

closed as off topic by mseancole, Brian Reichle, James Khoury, Jeff Vanzella, Corbin Sep 3 '12 at 5:34

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined in the FAQ. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about closed questions here.

1 Answer

up vote 1 down vote accepted

The trouble is that your check can fail. Before you can call the children method you need to be sure that you are dealing with an object (that provides the children method). This is actually what your fatal error is telling you ($current->AttributeSets is a non-object). You get this in your if statement within your isset call.

The trouble with fatal errors is that they are fatal. There is no way to recover from them. The PHP engine is about to shutdown. You can read more about handling errors in an answer I wrote here. The only way is to avoid fatal errors. Don't call functions that don't exist, or methods on something that is NULL.

Possibly the reason you have this problem is breaking the Law of Demeter. I don't know how the XML objects you are working with are built, etc.

You could solve it by checking each component.

Unfortunately you can't do this because isset can't use a function's return value:

// THIS WON'T WORK
isset($current->AttributeSets,
      $current->AttributeSets->children('ns2', true)) // Function Return Value

However as mseancole commented you can set the components as variables.

if (isset($current->AttributeSets))
{
    $children = $current->AttributeSets->children('ns2', true);

    if (isset($children,
              $children->ItemAttributes,
              $children->ItemAttributes->ListPrice, // etc.
share|improve this answer
does it matter if the information from the xml document is extracted as a simple_load_string in the function? I was trying your suggestion and it was not working. Thanks for any help. – Jim Aug 30 '12 at 14:37
sorry, i have fixed it. It basically becomes what mseancole wrote in the comment. @mseancole I'd be happy to upvote your comment if it was an answer (I think it really is very appropriate advice). – Paul Aug 31 '12 at 4:56

Not the answer you're looking for? Browse other questions tagged or ask your own question.