Hacker News new | ask | show | jobs
by BaconJuice 3538 days ago
Hi, can you list all the worst practices you see in this code? Just curious of the full list
2 comments

I'm no expert but since he didn't reply:

- Use of mysql_ functions (This extension was deprecated in PHP 5.5.0, and it was removed in PHP 7.0.0. Instead, the MySQLi or PDO_MySQL extension should be used)

- Using the user input directly in the query instead of binding, using prepared statements, or at the very least casting to an integer (don't do this)

- (besides mixing multiple layers) Possible whitespace problems in the output validation (if you're using JSON), you want most php code to be "validated" (like using json functions), to have proper headers, to output types correctly (like booleans). This is also why you should never use PHP's closing tag, except for things like pure PHP templating, it's even in the PSR-2:

> The closing ?> tag MUST be omitted from files containing only PHP.

Not really the case here but you get the idea. It's like you're using a "html view" to output JSON API stuff.

- Possible "Undefined index", or worse, at $ar["name"], $ar["location"]

- Having to write some ugly conditionals if you have more than a single result and need to end the sequence without a comma or do any kind of transformation

- Trying to save vertical space by using expressions in the same line, sacrificing readability

- No error handling. What if the DB connection fails?

- Output is not valid JSON

http://www.php-fig.org/psr/psr-2/

http://phptherightway.com/

http://jsonlint.com/

> The closing ?> tag MUST be omitted from files containing only PHP.

It doesn't contain only PHP. There's the closing JSON } after ?> :D

No input validation for one, possibility for SQL injection is another.