- Only use
include_once
orrequire_once
. There are very specific cases when you want to useinclude
orrequire
, connecting (to a database?) is not one of them. - Naming convention.
$variableNames
are always camelCased,ClassNames
are always CamelCaps. That's how I do it, adopt one and stick with it. - Your
MyFunctions
class should not be a class. It should be a bunch of functions. - That first condition is redundant. You're checking if any of the conditions are true, and then you check them one by one. You can skip the initial check.
- Please, don't use
mysql_*
functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial. - You are exposed to SQL Injection attacks. This is a critical issue. See How can I prevent SQL injection with PHP How can I prevent SQL injection with PHP .
- If your PHP file returns a JSON string, don't construct it yourself. You should only have one
echo
, at the bottom of the file, and that'secho json_encode($results);
. Everything you want to echo should be placed into the$results
array.
- Only use
include_once
orrequire_once
. There are very specific cases when you want to useinclude
orrequire
, connecting (to a database?) is not one of them. - Naming convention.
$variableNames
are always camelCased,ClassNames
are always CamelCaps. That's how I do it, adopt one and stick with it. - Your
MyFunctions
class should not be a class. It should be a bunch of functions. - That first condition is redundant. You're checking if any of the conditions are true, and then you check them one by one. You can skip the initial check.
- Please, don't use
mysql_*
functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial. - You are exposed to SQL Injection attacks. This is a critical issue. See How can I prevent SQL injection with PHP .
- If your PHP file returns a JSON string, don't construct it yourself. You should only have one
echo
, at the bottom of the file, and that'secho json_encode($results);
. Everything you want to echo should be placed into the$results
array.
- Only use
include_once
orrequire_once
. There are very specific cases when you want to useinclude
orrequire
, connecting (to a database?) is not one of them. - Naming convention.
$variableNames
are always camelCased,ClassNames
are always CamelCaps. That's how I do it, adopt one and stick with it. - Your
MyFunctions
class should not be a class. It should be a bunch of functions. - That first condition is redundant. You're checking if any of the conditions are true, and then you check them one by one. You can skip the initial check.
- Please, don't use
mysql_*
functions in new code. They are no longer maintained and are officially deprecated. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial. - You are exposed to SQL Injection attacks. This is a critical issue. See How can I prevent SQL injection with PHP .
- If your PHP file returns a JSON string, don't construct it yourself. You should only have one
echo
, at the bottom of the file, and that'secho json_encode($results);
. Everything you want to echo should be placed into the$results
array.
PHP
- Only use
include_once
orrequire_once
. There are very specific cases when you want to useinclude
orrequire
, connecting (to a database?) is not one of them. - Naming convention.
$variableNames
are always camelCased,ClassNames
are always CamelCaps. That's how I do it, adopt one and stick with it. - Your
MyFunctions
class should not be a class. It should be a bunch of functions. - That first condition is redundant. You're checking if any of the conditions are true, and then you check them one by one. You can skip the initial check.
- Please, don't use
mysql_*
functions in new code . They are no longer maintained and are officially deprecated . See the red box ? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial . - You are exposed to SQL Injection attacks. This is a critical issue. See How can I prevent SQL injection with PHP .
- If your PHP file returns a JSON string, don't construct it yourself. You should only have one
echo
, at the bottom of the file, and that'secho json_encode($results);
. Everything you want to echo should be placed into the$results
array.
PHP
- Only use
include_once
orrequire_once
. There are very specific cases when you want to useinclude
orrequire
, connecting (to a database?) is not one of them. - Naming convention.
$variableNames
are always camelCased,ClassNames
are always CamelCaps. That's how I do it, adopt one and stick with it. - Your
MyFunctions
class should not be a class. It should be a bunch of functions. - That first condition is redundant. You're checking if any of the conditions are true, and then you check them one by one. You can skip the initial check.
- Please, don't use
mysql_*
functions in new code . They are no longer maintained and are officially deprecated . See the red box ? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial . - You are exposed to SQL Injection attacks. This is a critical issue. See How can I prevent SQL injection with PHP .
- If your PHP file returns a JSON string, don't construct it yourself. You should only have one
echo
, at the bottom of the file, and that'secho json_encode($results);
. Everything you want to echo should be placed into the$results
array.
HTML
Lack of consistent naming conventions and code style. Sometimes you see
<script src= "js/drivers.js">
and sometimesid = 'drivers_name'
. Pick one, and stick with it. It makes your code that much more readable.Your
<form>
element lacks themethod=
andaction=
attributes.You have redundant code.
<label>
s must be associated with inputs. So the two labels at the top are not valid.You have too many HTML tags per input, don't forget that you can nest the input inside a label, thus associating it implicitly, and also saving you the need for using IDs for every single input. For example:
<label class="uk-form-label">Name <input type="text" name="driver_name" class="uk-form-small autocomplete_with_new_value_driver PascalCase SelectOnFocus width400"> </label>
You don't need
<div>
s for each label and each input inside of a label. Usedisplay: block;
on labels and inputs to your advantage!Don't use
<br>
s to control vertical spacing. Usemargin-top
andpadding-top
to your advantage!You are using redundant attributes, for example
<div align="center">
should be a div withtext-align: center;
set for it in a CSS stylesheet.tabindex
is redundant if you code your form properly with order of elements.The reset
<button type="reset">
is redundant. No one uses it for any real purpose (I can just refresh) and it's extremely annoying to try and hit the submit button, only to accidentally reset the form. Please don't.
JavaScript
- You don't need jQuery. At a glance, and especially since you've given IDs for everything, you don't really need jQuery for most of the things you do there.
- Leverage the use of HTML5's validation. Use Feature detection to determine when to use your validation, and when to let the browser do it for you. It's semantic, and native.
- Generalize the buttons. Why do you have 3 blocks of code for 3 buttons? They should all submit to the same page, with slightly different parameters. All processing is done at the server, client only displays the results.
HTML
Lack of consistent naming conventions and code style. Sometimes you see
<script src= "js/drivers.js">
and sometimesid = 'drivers_name'
. Pick one, and stick with it. It makes your code that much more readable.Your
<form>
element lacks themethod=
andaction=
attributes.You have redundant code.
<label>
s must be associated with inputs. So the two labels at the top are not valid.You have too many HTML tags per input, don't forget that you can nest the input inside a label, thus associating it implicitly, and also saving you the need for using IDs for every single input. For example:
<label class="uk-form-label">Name <input type="text" name="driver_name" class="uk-form-small autocomplete_with_new_value_driver PascalCase SelectOnFocus width400"> </label>
You don't need
<div>
s for each label and each input inside of a label. Usedisplay: block;
on labels and inputs to your advantage!Don't use
<br>
s to control vertical spacing. Usemargin-top
andpadding-top
to your advantage!You are using redundant attributes, for example
<div align="center">
should be a div withtext-align: center;
set for it in a CSS stylesheet.tabindex
is redundant if you code your form properly with order of elements.
HTML
Lack of consistent naming conventions and code style. Sometimes you see
<script src= "js/drivers.js">
and sometimesid = 'drivers_name'
. Pick one, and stick with it. It makes your code that much more readable.Your
<form>
element lacks themethod=
andaction=
attributes.You have redundant code.
<label>
s must be associated with inputs. So the two labels at the top are not valid.You have too many HTML tags per input, don't forget that you can nest the input inside a label, thus associating it implicitly, and also saving you the need for using IDs for every single input. For example:
<label class="uk-form-label">Name <input type="text" name="driver_name" class="uk-form-small autocomplete_with_new_value_driver PascalCase SelectOnFocus width400"> </label>
You don't need
<div>
s for each label and each input inside of a label. Usedisplay: block;
on labels and inputs to your advantage!Don't use
<br>
s to control vertical spacing. Usemargin-top
andpadding-top
to your advantage!You are using redundant attributes, for example
<div align="center">
should be a div withtext-align: center;
set for it in a CSS stylesheet.tabindex
is redundant if you code your form properly with order of elements.The reset
<button type="reset">
is redundant. No one uses it for any real purpose (I can just refresh) and it's extremely annoying to try and hit the submit button, only to accidentally reset the form. Please don't.
JavaScript
- You don't need jQuery. At a glance, and especially since you've given IDs for everything, you don't really need jQuery for most of the things you do there.
- Leverage the use of HTML5's validation. Use Feature detection to determine when to use your validation, and when to let the browser do it for you. It's semantic, and native.
- Generalize the buttons. Why do you have 3 blocks of code for 3 buttons? They should all submit to the same page, with slightly different parameters. All processing is done at the server, client only displays the results.