Reputation: 1817
After starting learning Codeigniter and thus getting a better handle at MVC I begun to wonder something.
Lets say there is a Model that controls users which are stored in a database. A simple registration is done through form which is validated in Control and then the data is passed on to the Model to be stored in database. Right now the process of bringing the posted data and setting up an array which to pass to the database to be stored is assembled at the Model like this:
function add_user() {
$new_user_data = array(
'etunimi' => $this->input->post('etunimi'),
'sukunimi' => $this->input->post('sukunimi'),
'osoite' => $this->input->post('osoite'),
'postinro' => $this->input->post('postinro'),
'toimipaikka' => $this->input->post('toimipaikka'),
'puhelin' => $this->input->post('puhelin'),
'email' => $this->input->post('email'),
'tunnus' => $this->input->post('tunnus'),
'salasana' => $this->input->post('salasana')
);
$insert = $this->db->insert('kayttajat', $new_user_data);
return $insert;
}
What I am considering is to transfer the assembly of the data into the Controller thus making the Model a little more independent and reusable. The final data would then be passed on as a method parameter thusly:
function add_user ($new_user_data) {
$insert = $this->db->insert('kayttajat', $new_user_data);
return $insert;
}
This to my understanding would distinquish more between the layers as the Model does nothing but retrieves and passes on the final information and the burden of proof and assembly is on the Controller while View just prints it all out and offers the UI.
I'd like some more experienced opinions on which concept is more akin to MVC principles and simply makes more sense.
Upvotes: 3
Views: 408
Reputation: 58444
The rant
This is why using CodeIgniter as example for doing MVC is such a horrid idea.In proper MVC the Model is a layer, which is made from multiple different classes, each dealing with one of several responsibilities. There should not be any class in model layer which contains or extends some class, that contains word "model" in name. That is the first sign, that you are doing it wrong.
The CI_Model
class by itself contains nothing. You cannot say that it is structure for containing domain business logic, because there is nothing in it. But the documentation urges you to use ActiveRecord pattern. It is a problem because pattern itself combines storage with business rules.
And it is called "model" for some reason.
You get group of classes (which for PR reasons are called "models"). And each of the classes deals exclusively with storage and logic of one table. There is no place for logic, that is based around having interaction between data from multiple tables.
This architectural flaw cause part of domain business logic (like validation) to leak into controllers. You end up creating structure, where, for entities to interact, there is no place in "model".
Stop trying to map your "models" to tables directly. Instead you should try to turn classes, that extend CI_Model
in something like service, which provide high level interface of domain business logic for the controllers. Such service-level structures could contain the interaction between entities.
These services then can employ the domain objects (which contain business logic for specific entity and can validate themselves) and something like data mappers for way to easily store and retrieve information of said entities.
Upvotes: 0
Reputation: 880
I'm a little experienced with CI & I can tell you that you logic is good. You can set the array with the insert values in the controller in order to make the model more reusable. In fact, if you wanna have a more general model you can use the following:
function general_insert($table,$data){
return $this->db->insert($table,$data);
}
So you just pass the name of the table and the data you want to insert.
Upvotes: 0
Reputation: 14747
Another option...
Controller
public function validate()
{
//apply the rules
$this->form_validation->set_rules('email', 'Email', 'required');
if ($this->form_validation->run() == FALSE)
{
$this->load->view('form_incompleted');
}
else
{
$email = $this->input->post('email');
//create the model and store it
$user = new User($email);
$user->add_user();
$this->load->view('form_completed');
}
}
Model
class User extends CI_Model {
var $email = '';
function __construct()
{
parent::__construct();
}
function __construct($email)
{
parent::__construct();
$this->email = $email;
}
/**
* Add the current user
*/
function add_user()
{
$this->db->insert('kayttajat', $this);
}
}
I have used just the email
attribute, but you can add all of them with no problems
Upvotes: 0
Reputation: 10996
Your second approach creates a rather pointless model function. You can achieve the same thing by just calling $this->db->insert()
.
The reason why the first case if "good" is because you'll only send given columns. If you're sloppy for instance, and send $this->input->post()
as parameter to your function, you're in bigger risk of getting mysel errors upon extra post fields.
My approach would be smiliar to this:
function add_user() {
$arr = array('etunimi', 'sukunimi', 'osoite', 'postinro', 'toimipaikka', 'puhelin', 'email', 'tunnus', 'salasana');
$new_user_data = array();
foreach($arr as $h)
$new_user_data[$h] = $this->input->post($h);
$insert = $this->db->insert('kayttajat', $new_user_data);
return $insert;
}
If you wish to pass the data, at least make sure only given fields are used:
function add_user($data) {
$arr = array('etunimi', 'sukunimi', 'osoite', 'postinro', 'toimipaikka', 'puhelin', 'email', 'tunnus', 'salasana');
$new_user_data = array();
foreach($arr as $h)
if (isset($data[$h]))
$new_user_data[$h] = $data[$h];
$insert = $this->db->insert('kayttajat', $new_user_data);
return $insert;
}
Upvotes: 1
Reputation: 14428
You can do the validation in the controller and send the data to the model:
Controller:
if ($this->form_validation->run()) {
$this->my_model->add_user($this->input->post());
}
Model:
function add_user($input) {
$this->db->insert('kayttajat', $input);
}
Upvotes: 1