Request for improvements

[treii28]

A couple of things I notice trying to implement this SDK. There seem to be some issues creating problems for me so I imagine they create problems for others.

first is the shared/AuthorizeNetTypes.php - at first I wondered why these exist as they seem to be redundant in that all of the types are included in the auto-generated class heirarchy from xsd2php under net/authorize/api/contract/v1 After trying to go directly to those under net/authorize/api/contract/v1 I quickly learned why the Types versions exist. Apparently the parser on Authorize's end doesn't convert the parsed XML to an object oriented structure. (it relies on sequential order of the elements in the incoming XML - all I can say to this is YUCK - seriously???) But ok, so one apparently needs to use these types to force the proper ordering via their addObject(). But the Types file includes multiple classes which means it can't be autoloaded as needed. I ended up breaking all of those types out to separate classes properly namespaced to allow for autoloading. This is something you might consider doing as well.

The second thing I ran into is that I wasn't interested in logging to a flatfile, but instead prefer to log to a database. No problem, I can facilitate this by creating my own versions of util/Log and util/LogFactory and then making sure my own version of Request accesses my version instead of the api version. The problem kicked in with the fact all of the properties and methods in Log and LogFactory are set to private. (specifically Log::log() and Log::getMasked() were what I needed) This means I cannot simply extend those classes and override the necessary functions because the access to the parent properties and classes are private. This can be remedied by setting them and Log::$sensitiveTags to protected instead of private. This (and perhaps others) is something else you might consider doing to allow for easier inheritance and overriding of your base classes. My only alternative now is to completely replace the files which also means I now must completely replace any files that access any of the functionality in those files, AND any time you now update the base classes, I have to spend more time making sure that any updates cascade to my versions manually.

Finally the Request and Response methods are also included in a single file. This isn't as much of a problem for the autoloading, as the Request class is always going to be imported when making the request, thus the Response class will already be loaded before the response is received. But you might consider breaking those two out also.

[ashtru]

@treii28 Thank you for your feedback, Scott. The classes in shared/ folder are legacy code. All the classes in net/authorize/api/contract/v1 inherit from ANetApiRequestType, and in that file and inherited classes, you will see different classes for each request/response type.

Allowing the extension of the Log/LogFactory is a good suggestion. It will be implemented soon.

Request and response methods are different for individual request/response types, and there are getters/setters for exposed properties. See here for a sample code, using the CreateTransactionRequest class.

Thanks again, and your suggestions are welcome.

[mahngiel]

@treii28

The problem kicked in with the fact all of the properties and methods in Log and LogFactory are set to private.

We also had this problem in #186 , which we put together a PR for.

Hopefully @ashtru 's team can make some movement on feedback.