Skip to content

Conversation

@danhunsaker
Copy link
Collaborator

  • Should throw exceptions on errors that prevent proper __cosntruct() execution.
  • Should throw exceptions on errors that prevent proper execution of other logic, unless actual error handling is implemented internally to such logic.

Also minimized operation overhead by moving initial node list loading to end of successful login, instead of on every action possible post-login. (There were checks to determine if upstream requests were required to fulfill this need, but that check itself presents an amount of overhead which can be undesirable in a high performance, high request load situation.)

Additionally, removed superfluous pve_ from property and method names. The fact that they are part of the PVE2_API class tells us that much of their nature. The fact that all were in protected/private scope makes the extra characters even more unnecessary.


NOTE: diff isn't very smart about detecting when newlines are added to existing text, so the first several changes (until about line 83 in the old/53 in the new) are less drastic than they appear. (git uses diff rather than replicate its functionality because Linus believes in not reinventing wheels he doesn't have to; git is obviously a wheel he had to 😁 )

Should throw exceptions on errors that prevent proper `__cosntruct()` execution.
Should throw exceptions on errors that prevent proper execution of other logic, unless actual error handling
    is implemented internally to such logic.

Also minimized operation overhead by moving initial node list loading to end of successful login, instead of
    on every action possible post-login.  (There were checks to determine if upstream requests were required
    to fulfill this need, but that check itself presents an amount of overhead which can be undesirable in a
    high performance, high request load situation.)

Additionally, removed superfluous 'pve_' from property and method names.  The fact that they are part of the
    PVE2_API class tells us that much of their nature.  The fact that all were in protected/private scope
    makes the extra characters even more unnecessary.

Signed-off-by: Daniel Hunsaker <danhunsaker@gmail.com>
@danhunsaker
Copy link
Collaborator Author

Oh, the other thing I forgot to mention in the commit message (and therefore in the PR above) is that I replaced occurrences of print() with error_log() since it's quite a bit cleaner than spitting out to STDOUT.

@CpuID
Copy link
Owner

CpuID commented Feb 2, 2015

Nice work, appreciate it. Sorry it took me a year to review and merge :)

CpuID added a commit that referenced this pull request Feb 2, 2015
Cleanup and update to PHP5 class conventions
@CpuID CpuID merged commit e4d7297 into CpuID:master Feb 2, 2015
@danhunsaker danhunsaker deleted the improve/PHP5-class-standards branch February 3, 2015 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants