How NOT to write (PHP) code

My comment on one of the files of SuiteCRM:

https://github.com/salesagility/SuiteCRM/issues/3687

So…

* todo: encapsulate logically related code into separate classes

— e.g. use (API) controller class(es)
— contact management API
— folder management API (create, rename, delete, etc.)
— email management API (read, compose drafts, send, relate to other entities etc.)
— signature management API
— attachment management API

* avoid: inconsistent behaviour/output: in some cases JSON, in others non-JSON output e.g. “NOOP”

* avoid: very long switch case block

* avoid: cases without break statements are generally confusing

* avoid: too many and too long if blocks

* avoid: relying on global variables e.g. $_REQUEST

* avoid: reading array items without checking index/key

* avoid: changing PHP settings: e.g. changing the threshold for execution time.

— the users do not wait for 5 minutes (300 seconds) in front of a browser, they just keep clicking every few seconds or so, or refresh the current page, navigate away to another page, etc. If PHP is setup to ignore user action, then it will continue processing a request until it finishes; and it means that there could be parallel threads for the same action by the same user and the code has to be re-architected to handle such scenarios.

— if there could be tasks which may take unknown amount of time (e.g. reading 500 unseen emails (with/without attachments) from inbox – see cases for checkEmail and checkEmail2), they should be performed by background workers connected to a proper queue service.

Leave a Comment