djekl
2/19/2015 - 6:20 PM

Ahhhhhhhhh!!!!

Ahhhhhhhhh!!!!

HOLY GAWD PEOPLE..

  • Don't write all your new code on the production machine!
  • Use a VCS!
  • Document your functions using DocBlocks!
  • Document your API (at least LIST the endpoints)... I have to decipher the meaning from your code, and that isn't really clear
  • Use a single identifier as the primary ID in the API for a resource (ie don't return a list of group ids for listing groups and then expect the group name as the identifying parameter for modifying a group)
  • Use a consistent model for representing the same data model in different endpoints!
  • Don't concatenate fields in the API results unless there is a good reason to (e.g first name and last name)
  • Don't expose the underlying database logic. This is leaky abstraction, especially when you do it inconsistently!!
  • Use HTTP codes to indicate request/response status! Don't add that information to the response body as a boolean field.
  • Even if you have to use a boolean field in the response, true = 1 and false = 0, not vice versa.
  • Don't rely on the client to implement business logic like validation!! That is going to lead to extremely complicated client code, and lead to errors and data inconsistencies.
  • There is absolutely no reason not to use HTTPS. We're passing potentially sensitive information here.
  • Don't require multiple HTTP calls for authorization when auth/access can be done in one step! Atomic things should be atomic.
  • The user login field should have one name and one name only. We call it login, unixaccount, and possibly others all over the place.
  • Put some thought into model attribute names! 'approved_comments' should be 'approver_comments', for example!
  • Consistency in naming is important: Don't use 'requestor' in some places and 'requester' in others
  • Limit the amount of potential entropy wherever possible. Don't allow fields to have any value if only a few values make sense! And definitely don't leave it up the clients to decide what those rules are!!!!
  • The server should handle data integrity, not the client! Eg. if a group request is fulfilled and it should change two tables in the database, don't make the clients make two separate calls; that is a recipe for data integrity violations!
  • A timestamp should be an actual timestamp down to hours/min/secs in ISO8601 format. A date is a 'date', not a timestamp!
  • Don't use completely different nouns in the system for the same object
  • If you expect me to send you the ID for a record, there should be at least one endpoint in the API where I can actually get it.
  • Do not mix data and logic. Hard-coding IDs in the code of mutable/deletable records from the database is a recipe for trouble.
  • The Request Signature algorithm should include a hash of the data being sent! Not just the timestamp and a secret, otherwise it is not secure at all.
  • Use relative paths for file includes! Or better yet, use an autoloader!

In the code

  • Don't Repeat Yourself! Holy moly.. 80% of the code is repeated overhead stuff, like getting a database connection
  • Don't manually create JSON strings by concatenating JSON syntax! Use arrays + json_encode function!
  • Configuration values should not be stored in the code! They should be stored in environment variables or a config file
  • Don't leave stray variables and other crud lying around in the codebase
  • Comments anywhere? Nope!