user5642240
user5642240

Reputation:

Simplify php user profile update code

I am making a user profile update php file and I wonder, is there any other ways to simplify this if statement code that checks if input is empty or not? Currently code looks like this:

`

<?php 
    session_start();
    $name = test_input($_POST['name']);
    $surname = test_input($_POST['surname']);
    $password = $_POST['password'];
    $newPassword = $_POST['newPassword'];
    $newConfirmPassword = $_POST['newConfirmPassword'];

    function test_input($data) {
        $data = trim($data);
        $data = stripslashes($data);
        $data = htmlspecialchars($data);
        $data = strtolower($data);
        return $data;
    };

    if($password != $_SESSION['password']){
        die('Wrong password, try again!');
    }else{
        if(empty($name) && empty($surname) && empty($newPassword) && empty($newConfirmPassword)){
            die('No changes has been made!');   
        }else if(!empty($name) && empty($surname) && empty($newPassword) && empty($newConfirmPassword)){
            //name update
        }else if(!empty($name) && !empty($surname) && empty($newPassword) && empty($newConfirmPassword)){
            //name and surname update
        }else if(!empty($name) && !empty($surname) && !empty($newPassword) && empty($newConfirmPassword)){
            die('New passwords do not match');
        }else if(!empty($name) && !empty($surname) && empty($newPassword) && !empty($newConfirmPassword)){ 
            die('New password do not match');
        }else if(!empty($name) && empty($surname) && !empty($newPassword) && !empty($newConfirmPassword)){
            //name and passwords update
        }else if(!empty($name) && !empty($surname) && !empty($newPassword) && !empty($newConfirmPassword)){
            //name , password and surname update
        }else if(empty($name) && !empty($surname) && empty($newPassword) && empty($newConfirmPassword)){
            //surname update
        }else if(empty($name) && !empty($surname) && !empty($newPassword) && empty($newConfirmPassword)){
            die('New passwords do not match');
        }else if(empty($name) && !empty($surname) && empty($newPassword) && !empty($newConfirmPassword)){
            die('New passwords do not match');
        }else if(empty($name) && !empty($surname) && !empty($newPassword) && !empty($newConfirmPassword)){
            //surname and password change
        }else if(empty($name) && empty($surname) && !empty($newPassword) && empty($newConfirmPassword)){
            die('New passwords do not match');
        }else if(empty($name) && empty($surname) && empty($newPassword) && !empty($newConfirmPassword)){
            die('New passwords do not match');
        }else if(empty($name) && empty($surname) && !empty($newPassword) && !empty($newConfirmPassword)){
            //password change
        };
    };
?>

`

Maybe there is a way to do this with function somehow?

Upvotes: 1

Views: 91

Answers (4)

Peter VARGA
Peter VARGA

Reputation: 5186

I think John Slegers' answer is very good. It is the way how a program should work.

In my opinion it can be implemented even more "elegant" with exceptions. The advantage when using exceptions is, having nested functions, you do not have to check in each level the return value and pass it through - instead of this you can implement the final flow at only one point and it is well defined.

Think about spend around 33$ for this book Clean Code by Martin. There are some good ideas. You may not agree with all of them but it gives you really a good guideline how to program.

As John pointed out in his comment try to use multiple functions and give the functions very good names. Be happy you are not a programmer 30 years ago where you had 2 characters for your function and variables names...

It is a very good sign you asked this question because you were not "satisfied" with your version.

Upvotes: 0

John Slegers
John Slegers

Reputation: 47101

I would use multiple functions : one to see if no changes are made at all, one to test if the passwords are valid, etc.

Example code :

function stripName($data) {
    return strtolower(htmlspecialchars(stripslashes(trim($data)));
}

function noChanges($name, $surname, $newPassword, $newConfirmPassword) {
    if(empty($name) &&
       empty($surname) &&
       empty($newPassword) &&
       empty($newConfirmPassword)
    ) {
        return 'No changes has been made!';
    }
    return false;
}

function oldPasswordIsInvalid($password) {
    if($password !== $_SESSION['password']){
         return 'Wrong password, try again!';
    }
    return false;
}

function newPasswordIsInvalid($password1, $password2) {
    if(empty($password1) {
        return 'Password must not be empty';
    }
    if(empty($password2) {
        return 'Password confirmation must not be empty';
    }
    if($password1 !== $password2) {
        return 'Passwords do not match';
    }
    return false;
}

function errorFound(
        $name,
        $surname,
        $password,
        $newPassword,
        $newConfirmPassword
) {
    return 
        oldPasswordIsInvalid($password) || 
        noChanges($name, $surname, $newPassword, $newConfirmPassword) || 
        newPasswordIsInvalid($newPassword, $newConfirmPassword);
}

function removeEmptyFieldsFromArray($array) {
    $returnArray = [];
    foreach($array as $key => $value) {
        if(!empty($value)) {
            $returnArray[$key] = $value;
        }
    }
    return $returnArray;
}

session_start();
$name = stripName($_POST['name']);
$surname = stripName($_POST['surname']);
$password = $_POST['password'];
$newPassword = $_POST['newPassword'];
$newConfirmPassword = $_POST['newConfirmPassword'];

if ($errorMessage = errorFound(
        $name,
        $surname,
        $password,
        $newPassword,
        $newConfirmPassword
)) {
    die($errorMessage);
} else {
    $valuesThatNeedToBeUpdated = removeEmptyFieldsFromArray([
        'name' => $name,
        'surname' => $surname,
        'password' => $newPassword
    ]);
    // $valuesThatNeedToBeUpdated is an array that contains
    // only the fields that aren't empty.
    // Fields in this array are the ones that need to be updated
}

Upvotes: 0

apokryfos
apokryfos

Reputation: 40681

session_start();
if($_POST['password'] !== $_SESSION['password']){
    die('Wrong password, try again!');
}
$data['name'] = test_input($_POST['name']);
$data['surname'] = test_input($_POST['surname']);
$passwordConfirmed = $_POST['newConfirmPassword'] === $_POST['newPassword'] && !empty($_POST['newPassword']);
$newPassword = $_POST['newPassword'];
$data = array_filter($data);
if (count($data)===0) { 
   die('No changes has been made!'); 
} 
if (!empty($newPassword) && $passwordConfirmed) {
   //Update password
} else {
   foreach ($data as $field => $value) {
        //Update field. This would work best if your $field variable makes updating easy to code.
       //break; //If you only want to update one field on each request
   }
}

This looks slightly smaller.

Upvotes: 1

Fabian N.
Fabian N.

Reputation: 1240

You can write a function who can update a) The name b) The surname c) the password. After that you can do this:

if(!empty($name)){
  updateName($name);
}
if(!empty($surname)){
  updateSurname($surname);
}
if(!empty($newPassword) && !empty($newConfirmPassword)){
  if($newPassword == $newConfirmPassword){
    updatePassword($newPassword);
  }
}

Upvotes: 0

Related Questions