patternjavascriptMinor

Driver License Input Form

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

inputformlicensedriver

Problem

I am willing to switch to OOP concept, but before working on that I would like to know whether what I have working so far is OK, or if any major change is required?

For every form I create 3 files:

for front end html

a js file as a mediator between client and server

a server side php file which handles data services.

Please help me improve my approach.


    

    
    Drivers

    

        
            Name
            
                
            
        

        
            Re-Name
            
                
            
        

        
            Present Address
            
                 
            
        

        
            Perm. Address
            
                 
            
        

        
            Licence No.
            
                
            
        

        
            Licence Valid Upto
            
                
            
        

        
        
            Save
            Reset
            Delete

```
// ===============================================================
// Second .JS File : drivers.js
// ===============================================================
$(document).ready(function() {
//------------------------------------------------------------------------------ Date Picker
$(function() {
$(".datePicker").datepicker({dateFormat: 'dd-mm-yy'});
});

//-------------------------------------------------------------------------- Plugin For Combobox with new values
var $input = $(".autocomplete_with_new_value_driver").autocomplete({
source: "php/driverslist.php?nameonly=Y",
minLength: 0
}).addClass("no-border-right ");

$(" ")
.attr("tabIndex", -1)
.attr("title", "Show All Items")
.insertAfter($input)
.button({
icons: {
primary: "ui-icon-triangle-1-s"

Solution

HTML

Lack of consistent naming conventions and code style. Sometimes you see ` and sometimes id = 'drivers_name'. Pick one, and stick with it. It makes your code that much more readable.

Your element lacks the method= and action= attributes.




- 
You have redundant code.

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:

Name

You don't need s for each label and each input inside of a label. Use display: block; on labels and inputs to your advantage!

Don't use

s to control vertical spacing. Use margin-top and padding-top

 to your advantage!

You are using redundant attributes, for example should be a div with text-align: center; set for it in a CSS stylesheet. tabindex is redundant if you code your form properly with order of elements.

The 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.



PHP

Only use include_once or require_once. There are very specific cases when you want to use include or require, 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's echo json_encode($results);. Everything you want to echo should be placed into the $results` array.

Code Snippets

<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>

Context

StackExchange Code Review Q#52703, answer score: 4

Revisions (0)

No revisions yet.