Account-Creation form

Question

How can I optimize the following PHP/JS code?

It works like a charm but I think it is too long and I'm sure it could be optimized. If I have another form, I'll need to copy paste some lines from my PHP code and it'll be quickly overkill.

---

JavaScript:

<script>
$(document).ready(function() {
    $('#createAccount').on('submit', function() {
        var Email = $('#Email').val();
        var Password = $('#Password').val();
        var PasswordRepeat = $('#PasswordRepeat').val();

        $.ajax({
            url: $(this).attr('action'),
            type: $(this).attr('method'),
            data: $(this).serialize(),
            dataType: 'json',
            success: function(json) {
                $('.alert-message').text(json.reponse).show();
            }                           
    });
    return false;
    });
});
</script>

PHP:

<?php
$reponse="";
if(isset($_POST['Email']) && isset($_POST['Password']) && isset($_POST['PasswordRepeat'])) {

    // Si tous les champs ne sont pas renseignés
    if($_POST['Email'] == '' || $_POST['Password'] == '' || $_POST['PasswordRepeat'] == '') {
            $reponse = "Tous les champs doivent être remplis.";
    }

    // Si les mots de passe ne sont pas similaires
    if($_POST['Password'] != $_POST['PasswordRepeat']) {
            $reponse = "Les mots de passe ne sont pas similaires.";
    }

    // Si ce n'est pas une adresse de courriel
    if(!preg_match("/^([a-zA-Z0-9])+([a-zA-Z0-9\._-])*@([a-zA-Z0-9_-])+([a-zA-Z0-9\._-]+)+$/", $_POST['Email'])) {
            $reponse = "L'adresse de courriel ne semble pas conforme.";
    }

    // Si le mot de passe est trop court
    if(strlen($_POST['Password']) <= 5) {
            $reponse = "Le mot de passe être top court.";
    }

    // Si tout est ok
    if(($_POST['Password'] == $_POST['PasswordRepeat']) && ($_POST['Email'] != '' && $_POST['Password'] != '' && $_POST['PasswordRepeat'] != '') && (preg_match("/^([a-zA-Z0-9])+([a-zA-Z0-9\._-])*@([a-zA-Z0-9_-])+([a-zA-Z0-9\._-]+)+$/", $_POST['Email'])) && (strlen($_POST['Password']) >= 6)) {
            $reponse = "Nice";
    }

}
else {
    $reponse = "Une erreur s'est produite.";
}

$array['reponse'] = $reponse;
echo json_encode($array);
?>

Answer 1

At least for the PHP part:

You should make it into a function where you return status instead of going over every possibility even if you have a fail.

function validate_post() {
   if ( ... )
      return "no";
   if ( ... )
      return "yes";
}

$array['reponse'] = validate_post();

By this you would eliminate the last // Si tout est ok part as well, as by the response would be OK.

---

If you truly want to eliminate possible steps, you could also negate the isset() checks and use OR instead of AND, as it then will stop on first false.

if (!isset($_POST['Email']) || !isset($_POST['Password']) ... )
   return "Une erreur s'est produite.";

The length check for password require less resources and as such could be moved above the email check. (Which would be the most expensive.)

---

For the Javascript part

it becomes a question on what and if you need to optimize.

It is likely a process rarely taken. Manual action by user (hopefully) not in a loop ;). When it get submitted you are to have a rather big form for it to have a noticeable effect.

That said jQuery gives you a overhead, but you might/likely do not want to use pure Javascript.

---

Edit:

Had another look at this, and also had to check if I was missing something regards to your variables in the Javascript. But, unless you have some omitted code within the "submit" function – you have some unused variables:

var Email = $('#Email').val();
var Password = $('#Password').val();
var PasswordRepeat = $('#PasswordRepeat').val();

As you use:

data : $(this).serialize(),

Which in effect serializes all form fields with a name attribute, and not something like:

data : $.param({
          Email : Email,
          Password : Email,
          PasswordRepeat : PasswordRepeat
       }),

By which you could also use:

data : $.param({
    Email : $('#Email').val(),
    Password : $('#Password').val(),
    PasswordRepeat : $('#PasswordRepeat').val()
}),

It depends on use. If for some reason your form has lot of fields that you do not want to post the latter would be to prefer, though that would also bind it much tighter together and make it a possible spot for bugs in future changes to the code.

---

As a side-note you also should most likely have this:

$('#form').on('submit', function(event) {
    event.preventDefault();  // Prevent form submit.
    ...