I've been using CodeIgniter for a while, and have a decent knowledge of MVC, PHP etc.
However, I'm finding it hard to adhere to the Fat Model Skinny Controller ethos.
I've seen a lot about it; including what pseudo code to include in each file, but no actual examples. (Please link to some articles if I've missed any obvious ones!)
I'm finding it hard to move the form logic to a model. For instance, I am using a custom library for my auth system, which has it's own model. Should I then make a site user Model to log users in? Or should I just make a site Model to do that? Or a form Model?
To help me out, can anyone advise me on how to skinnify this Controller? I realise it's a lot of code, but simple pointers would be great. (Please note, I've only just written this code, so it hasn't been refactored much, but it should give a good example of how some of my methods are getting out of hand.)
public function register()
{
session_start();
if ($this->tf_login->logged_in())
{
redirect('profile');
}
if ($_GET['oauth'] == 'true')
{
$type = $_GET['type'];
try
{
$token = $this->tf_login->oauth($type, '', 'email');
}
catch (TFLoginCSRFMismatchException $e)
{
$this->tf_assets->add_data('error_message', $e->getMessage());
}
catch (TFLoginOAuthErrorException $e)
{
$this->tf_assets->add_data('error_message', $e->getMessage());
}
if ($token)
{
$user_details = $this->tf_login->call('https://graph.facebook.com/me?fields=email,first_name,last_name,username&access_token=' . $token);
$user_details_decoded = json_decode($user_details);
if ($user_details_decoded->email)
{
try
{
$id = $this->tf_login->create_user($user_details_decoded->username,
md5($user_details_decoded->username . time()),
$user_details_decoded->email,
'',
TRUE,
TRUE);
}
catch (TFLoginUserExistsException $e)
{
try
{
if ($this->tf_login->oauth_login($type, $user_details_decoded->email, $token))
{
$this->session->set_flashdata('success_message', 'You have successfully logged in.');
redirect('profile');
}
else
{
$this->session->set_flashdata('error_message', 'An account with these details exists, but currently isn\'t synced with ' . $type . '. Please log in to sync the account.');
}
}
catch (Exception $e)
{
$this->session->set_flashdata('error_message', $e->getMessage());
}
}
catch (TFLoginUserNotCreated $e)
{
$this->tf_assets->add_data('error_message', 'You could not be registered, please try again.');
}
if ($id)
{
$this->tf_login->add_user_meta($id, 'first_name', $user_details_decoded->first_name);
$this->tf_login->add_user_meta($id, 'surname', $user_details_decoded->last_name);
$this->tf_login->sync_accounts($id, $type, $token);
$this->session->set_flashdata('success_message', 'Welcome ' . $this->input->post('first_name', TRUE) . ' ' . $this->input->post('surname', TRUE) . '. Your account has been sucessfully created. You will shortly receive an email with a verification link in.');
redirect('login');
}
}
else
{
$this->session->set_flash_data('error_message', 'You could not be logged in, please try again.');
}
}
// Redirect to clear URL
redirect(current_url());
}
if ($this->form_validation->run() !== FALSE)
{
try
{
$id = $this->tf_login->create_user($_POST['username'], $_POST['password'], $_POST['email'], '', FALSE);
}
catch (Exception $e)
{
$this->tf_assets->add_data('error_message', $e->getMessage());
}
if ($id)
{
$this->tf_login->add_user_meta($id, 'first_name', $_POST['first_name']);
$this->tf_login->add_user_meta($id, 'surname', $_POST['surname']);
if ($this->tf_login->register_verification_email())
{
$this->session->set_flashdata('success_message', 'Welcome ' . $this->input->post('first_name', TRUE) . ' ' . $this->input->post('surname', TRUE) . '. Your account has been sucessfully created. You will shortly receive an email with a verification link in.');
redirect('login');
}
else
{
$this->tf_login->login_user($id);
$this->session->set_flashdata('success_message','Your account has been sucessfully created.');
redirect('profile');
}
}
else
{
$this->tf_assets->add_data('error_message', $this->tf_login->get_errors());
}
}
if (validation_errors())
{
$this->tf_assets->add_data('error_message', validation_errors());
}
$this->tf_assets->set_content('public/register');
$this->tf_assets->add_data('page_title', "Register");
$this->tf_assets->render_layout();
}
Thanks in advance!
From what I can tell, most or all of this code belongs in a controller or component, so I don't think your problem is Model/Controller confusion.
The code is difficult to read, however, because of the deep nested structures and the failure to break out specific tasks into their own methods. The main refactoring you would benefit from here is creating new private methods to separate out the discrete subtasks that you are performing. This is has the additional important benefit of clarifying the high-level structure of your current method. So you would end up with something that looked like (just to give you a rough example):
public function register()
{
session_start();
if ($this->tf_login->logged_in())
{
redirect('profile');
}
if ($_GET['oauth'] == 'true')
{
$this->oauthRegister();
}
$this->normalRegister();
}
Similarly, the oatuhRegister
method and normalRegister
methods would be broken down into smaller methods themselves, so that when you were completely finished each method would be adhering to the SRP and would probably be fewer than 10 lines of code. This will drastically improve the readability and maintainability of your code. I'd also recommend checking out Clean Code, which makes a strong argument for keeping your method short.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With