Skip to content
This repository has been archived by the owner on Nov 26, 2017. It is now read-only.

Form Package: db->error, Exception/JText modifications #1090

Merged
merged 1 commit into from
Apr 3, 2012
Merged

Form Package: db->error, Exception/JText modifications #1090

merged 1 commit into from
Apr 3, 2012

Conversation

AmyStephen
Copy link
Contributor

No description provided.

// TODO: throw exception.

return $default;
throw new RuntimeException('XML is not an instance of JXMLElement');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would throw this as an UnexpectedValueException like this:

throw new UnexpectedValueException(sprint('%s::getFieldAttribute `xml` is not an instance of JXMLElement', get_class($this)));

That way it's easier for the developer to home in on where the error is. Ditto for the other cases.

@eddieajau
Copy link
Contributor

Really good job overall Amy, just a bit of nuance to fix.

@AmyStephen
Copy link
Contributor Author

All right, I think I have this one updated for @eddieajau 's feedback.

Thanks Andrew.

@eddieajau eddieajau merged commit 7021239 into joomla:staging Apr 3, 2012
@eddieajau
Copy link
Contributor

Merged. Thanks.

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

Successfully merging this pull request may close these issues.

2 participants