PHP Question

PHP
1

Hi,

Developing an application (must be about the 3rd time I have recoded it) and just have a question about the best way to go about it.

For example I will use my register section (I understand some inputs havent been cleaned as of yet).


        if($_POST['do'] == "register")
        {
            if(_empty($_POST['username'], $_POST['password'], $_POST['passwordConfirmation'], $_POST['email']))
            {
                $template->assign('message', array('type' => 'Error', 'content' => 'Please go back and ensure all fields are filled in.'));
                $template->assign('template', 'message');
            }
            elseif($core->db->fetch_row("SELECT `user_id` FROM `users` WHERE `username` = '{$_POST['username']}' OR `email` = '{$_POST['email']}'"))
            {
                $template->assign('message', array('type' => 'Error', 'content' => 'The username or email you are trying to use already exists in the database.'));
                $template->assign('template', 'message');
            }
            elseif($_POST['password'] != $_POST['passwordConfirmation'])
            {
                $template->assign('message', array('type' => 'Error', 'content' => 'The passwords you entered do not match.'));
                $template->assign('template', 'message');
            }
            elseif(!$core->db->insert("users", array('username' => mysql_real_escape_string($_POST['username']), 'password' => md5($_POST['password']), 'email' => mysql_real_escape_string($_POST['email']))))
            {
                $template->assign('message', array('type' => 'Error', 'content' => 'Something went wrong, please go back and try again.'));
                $template->assign('template', 'message');
            }
            else
            {
                $template->assign('message', array('type' => 'Success', 'content' => 'You have registered.'));
                $template->assign('template', 'message');
            }

This relies on my DB class and some of the functions I have written for it. Now, should I be doing something like this instead (the above works, but I want to know the best way to approach this).

        if($_POST['do'] == "register")
        {
            
            if(_empty($_POST['username'], $_POST['password'], $_POST['passwordConfirmation'], $_POST['email']))
            {
                $template->assign('message', array('type' => 'Error', 'content' => 'Please go back and ensure all fields are filled in.'));
                $template->assign('template', 'message');
            }
            else
            {
                $register = $core->auth->register($_POST['username'], $_POST['password'], $_POST['passwordConfirm'], $_POST['email']);
                
                if($register == "USERNAME_EXISTS")
                {
                    $template->assign('message', array('type' => 'Error', 'content' => 'The email you entered already exists.'));
                    $template->assign('template', 'message');
                }
                elseif($register == "EMAIL_EXISTS")
                {
                    $template->assign('message', array('type' => 'Error', 'content' => 'The username you entered already exists.'));
                    $template->assign('template', 'message');
                }
                elseif($register == "SUCCESS")
                {
                    $template->assign('message', array('type' => 'Error', 'content' => 'You have registered..'));
                    $template->assign('template', 'message');
                }
            }

Then my Auth class register function will have pretty much the same code as my first bit of code, and return either USERNAME_EXISTS, EMAIL_EXISTS, SUCCESS etc.

It looks a bit cleaner in the user.php page and moves some of the messy code to the auth class file, but is there any advantage of doing this?

Hope that makes sense.

Ryan

2

The advantage that I see is that you can now use a switch statement to retrieve the message from a message class.


$errorMessage = array(
    'FILL_ALL_FIELDS' => 'Please go back and ensure all fields are filled in.',
    'USERNAME_EXISTS' => 'The email you entered already exists.',
    ...
);
if($_POST['do'] == "register") {
    if (_empty($_POST['username'], $_POST['password'], $_POST['passwordConfirmation'], $_POST['email'])) {
        $errorType = 'FILL_ALL_FIELDS';
    } else {
        $errorType = $core->auth->register($_POST['username'], $_POST['password'], $_POST['passwordConfirm'], $_POST['email']);
    }
    $template->assign('message', array('type' => 'Error', 'content' => $errorMessage[$errorType]));
    $template->assign('template', 'message');
}

You might even want to put together an error class, so that you can extend it to add different messages.