consolibyte / quickbooks-php

QuickBooks Integration for PHP
Eclipse Public License 1.0
458 stars 338 forks source link

Cleanup asQBXML() & some undefined constants. #321

Open amorsent opened 1 year ago

amorsent commented 1 year ago

Overview:

Note: This is a simplified and more focused version of #194. My version stays focused on the asQBXML() method for now, and I've kept the commits clean and hopefully easy to follow. However, @cmancone is correct that _cleanup() and asArray() have similar issues.

Why are the subclassed implementations Redundant? These copies all simply invoke the _cleanup() method and delegate back to the central QuickBooks_QBXML_Object::asQBXML() which already calls the _cleanup() method. _Also note: in most cases cleanup() is an empty function anyway.

Many of these methods are also using incorrect function arguments and undefined constants. $todo_for_empty_elements was passed into the $version argument of the parent method. The default value QUICKBOOKS_OBJECT_XML_DROP is undefined and PHP interprets this as the string literal 'QUICKBOOKS_OBJECT_XML_DROP'. This hasn't caused issues because that argument is effectively unused by the parent method. (There is an if block that checks it, but it does nothing)

$indent was being passed into $locale in the parent method. The default value "\t" is technically one character and the parent method ignores anything that isn't exactly 2 characters.

Effect of dropping the method overrides:

Related: There are several other issues and pull requests related undefined constants. This pull request would at least partially address many of them.

Here's a few:

15

126

127

185

194

260