3 Replies - 1327 Views - Last Post: 23 January 2013 - 06:26 PM Rate Topic: -----

#1 xtremer360  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 123
  • Joined: 03-March 11

Single Responsibility Principal

Posted 22 January 2013 - 01:19 PM

I'm wanting to work on separating this submit function so that it doesn't do too much. What can I do to make this functions into smaller functions.

/* Not sure if this is needed
if ($this->session->userdata('failed_logins'))
{
    // User has previous failed logins in session
    $failed_logins = $this->session->userdata('failed_logins');
}
*/

 /**
 * submit function.
 * 
 * @access public
 * @param string $post_username
 * @param string $post_password
 * @return json data string
 */
public function submit($post_username = NULL, $post_password = NULL)
{
    // Set variable defaults
    $output_status = 'Notice';
    $output_title = 'Not Processed';
    $output_message = 'The request was unprocessed!';

    // Set validation rules for post data
    $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');
    $this->form_validation->set_rules('password', 'Password', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');
    $this->form_validation->set_rules('remember', 'Remember Me', 'trim|xss_clean|integer');

    if ($this->form_validation->run() == TRUE)
    {
        // Form validation passed

        // Number of error flags
        $x = 0;

        // Post values from login form
        $post_username = $this->input->post('username');
        $post_password = $this->input->post('password');

        // Get user data from post username value
        $user_data = $this->users_model->get_users(array('usernames' => $post_username), 'row');
        //print_r($user_data);
        //die();

        if ($user_data !== NULL)
        {
            // User was found in database

            if ($user_data->lock_date !== '0000-00-00 00:00:00')
            {
                // User is locked out

                // Get the current GMT time
                $current_time = now();

                if (strtotime($user_data->lock_date) > $current_time)
                {
                    // User is still locked out

                    $output_status = 'Error';
                    $output_title = 'Account Locked';
                    $output_message = 'This user account is current locked!';
                    $x++;
                }
                else
                {
                    // User can be unlocked and form be resubmitted

                    $this->users_model->unlock_user($user_data->user_id);
                    $this->submit($post_username, $post_password);
                    return FALSE;
                }
            }

            if ($x == 0)
            {
                // No error flags reported

                // User is not locked out

                if ($user_data->user_status_id == 1)
                {
                    $output_status = 'Error';
                    $output_title = 'Account Unverified';
                    $output_message = 'Sorry you must verify your account before logging in!';
                    $x++;
                }

                if ($user_data->user_status_id == 3)
                {
                    $output_status = 'Error';
                    $output_title = 'Account Suspended';
                    $output_message = 'Your account has been suspended!';
                    $x++;
                }

                if ($user_data->user_status_id == 4)
                {
                    $output_status = 'Error';
                    $output_title = 'Account Banned';
                    $output_message = 'Your account has been banned!';
                    $x++;
                }

                if ($user_data->user_status_id == 5)
                {
                    $output_status = 'Error';
                    $output_title = 'Account Deleted';
                    $output_message = 'Your account has been deleted!';
                    $x++;
                }

                if ($x == 0)
                {
                    // No error flags reported

                    // User is registered and verified

                    $regenerated_post_password = $this->functions_model->regenerate_password_hash($post_password, $user_data->password_hash);

                    /* Not sure if this is needed
                    if ($this->session->userdata('failed_logins'))
                    {
                        // User has previous failed logins in session
                        $failed_logins = $this->session->userdata('failed_logins');
                    }
                    */

                    if ($regenerated_post_password == $user_data->password)
                    {
                        // Password from login form matches user stored password
                        // Set session variable with user id and clear previous failed login attempts

                        $this->session->set_userdata('xtr', $user_data->user_id);
                        $this->session->unset_userdata('failed_logins');
                        $output_status = 'Success';
                        $output_title = 'Login Success';
                        $output_message = 'Successful login! Sending you to the dashboard';
                    }
                    else
                    {
                        // Password from login from does not match user stored password

                        if (is_integer($failed_logins))
                        {
                            // User has atleast one failed login attempt for the current session

                            if ($failed_logins == 4)
                            {
                                $wait_time = 60 * 15;
                                $lock_out_time = $current_time + $wait_time;

                                /* Find out about if I can do this part differently.
                                $lock_out_date = gmdate('Y-m-d H:i:s', $lock_out_time); 
                                */

                                $this->users_model->lock_out_user($user_data->user_id, $lock_out_date);
                                //$this->functions_model->send_email('maximum_failed_login_attempts_exceeded', $user_data->email_address, $user_data)
                                $output_status = 'Error';
                                $output_title = 'Account Locked';
                                $output_message = 'Your account is currently locked, we apologize for the inconvienence. You must wait 15 minutes before you can log in again! An email was sent to the owner of this account! Forgotten your username or password? <a href="forgotusername">Forgot Username</a> or <a href="forgotpassword">Forgot Password</a>';
                            }
                            else
                            {
                                // User has a few more chances to get password right

                                $failed_logins++;
                                $this->session->set_userdata('failed_logins', $failed_logins);
                                $output_status = 'Error';
                                $output_title = 'Incorrect Login Credentials';
                                $output_message = 'Incorrect username and password credentials!';
                            }
                        }
                        else
                        {
                            // First time user has not entered username and password correctly

                            $this->session->set_userdata('failed_logins', 1);
                            $output_status = 'Error';
                            $output_title = 'Incorrect Login Credentials';
                            $output_message = 'Incorrect username and password credentials!';
                        }

                        $time_of_attempt = gmdate('Y-m-d H:i:s');
                        $this->users_model->increase_login_attempt($this->input->ip_address(), $post_username, $time_of_attempt);
                    }
                } // if ($x = 0) User is registered and verified
            } // if ($x = 0) User is not locked out
        }
        else
        {
            // User was not found in database

            $output_status = 'Error';
            $output_title = 'User Not Found';
            $output_message = 'The user was not found in the database!';
        }
    }
    else
    {
        // Form validation failed
        $output_status = 'Error';
        $output_title = 'Form Not Validated';
        $output_message = 'The form did not validate successfully!';
    }

    echo json_encode(array('output_status' => $output_status, 'output_title' => $output_title, 'output_message' => $output_message, 'error_messages' => $this->form_validation->error_array()));
}




This is what I have so far...

public function form_is_valid()
	{
		$this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');
		$this->form_validation->set_rules('password', 'Password', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');

		return $this->form_validation->run();
	}
	
	public function submit()
	{
		$output_status = 'Notice';
		$output_title = 'Not Processed';
	    $output_message = 'The request was unprocessed!';
	    
	    if ($this->form_is_valid())
	    {
		    $output_status = 'Success';
		    $output_title = 'Form Submitted Successfully';
		    $output_message = 'The form did validate successfully!';
	    }
	    else
	    {
		    $output_status = 'Error';
		    $output_title = 'Form Not Validated';
		    $output_message = 'The form did not validate successfully!';
	    }
		
		echo json_encode(array('output_status' => $output_status, 'output_title' => $output_title, 'output_message' => $output_message, 'error_messages' => $this->form_validation->get_error_array()));
	}



Is This A Good Question/Topic? 0
  • +

Replies To: Single Responsibility Principal

#2 xtremer360  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 123
  • Joined: 03-March 11

Re: Single Responsibility Principal

Posted 23 January 2013 - 10:46 AM

Any ideas?
Was This Post Helpful? 0
  • +
  • -

#3 BetaWar  Icon User is online

  • #include "soul.h"
  • member icon

Reputation: 1155
  • View blog
  • Posts: 7,171
  • Joined: 07-September 06

Re: Single Responsibility Principal

Posted 23 January 2013 - 12:13 PM

Well, it is a rather large amount of code there, but I would suggest instead of having and setting the three variables (output message, status, and title) for every possible if/else condition you instead make that another object and simply return one of them (or JSON encode one of them) at the end of the function.

Then, there are a few different ways you could do this. If you know what every possible return value is at the time of programming (IE none of the text is dynamically generated) you could have your functions return a return code stating what happened when validating portions (for example 1 if the user is banned, 2 if suspended, etc.) and use a lookup table to gather the information you need for the submit() function's return value. This could get a little tedious, and potentially confusing if you have to add new messages later on.

For that reason, I would suggest simply splitting out all the functions according to what they do. For example, you check if the account is suspended, banned, etc. Move all of those if statements to a separate function that returns one of the messaging objects I talked about in paragraph 1, or NULL if there is no message to display. If the returned message is NULL it should be enough to signal you to continue with the submission validation; otherwise you should probably stop there and return the JSON information.

An example of what this may look like could be:
public function form_is_valid(){
  $this->form_validation->set_rules('username', 'Username', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');
  $this->form_validation->set_rules('password', 'Password', 'trim|required|xss_clean|min_length[6]|max_length[12]|regex_match[/[a-z0-9]/]');

  if(!$this->form_validation->run()){
    return array("output_status" => "Error", "output_title" => "Form Not Validated", "output_message" => "The form did not validate successfully!");
  }
  return NULL;
}

public function submit($post_username = NULL, $post_password = NULL){
  $outputStuff = array("output_status" => "Notice", "output_title" => "Not Processed", "output_message" => "The request was unprocessed!");
  if(($outputStuff = form_is_valid()) != NULL){
    return json_encode($outputStuff);
  }
  if(($outputStuff = user_is_valid()) != NULL){ // I didn't implement this one, but you should be able to get the idea.
    return json_encode($outputStuff);
  }
  // etc ...
  return json_encode($outputStuff);
}


This code updated the output information based on each function that passes running. For every function that fails, you get a non-NULL value back and are able to return that right then and there. If there are only a few instances of function to run for the submission process, then you can easily do each one as I showed above, however if you are using a LOT of functions to get through the submission process, you would likely be best coming up with a standard array/hash (associative array) that you pass to each function, place each validation function in an array, and loop through each one calling it with the default argument. That will greatly reduce the amount of code necessary.

Hope that helps.
Was This Post Helpful? 1
  • +
  • -

#4 xtremer360  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 123
  • Joined: 03-March 11

Re: Single Responsibility Principal

Posted 23 January 2013 - 06:26 PM

That's great. Any additional ideas from anyone?
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1