• 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Need refactoring advice

#1
Hello, all.

Here is my model superclass (used by several child models):

PHP Code:
<?php

class MY_Model extends CI_Model
{

    protected 
$table '';

    protected function 
switch_db($webinar_id)
    {
        if (! 
$this->db->db_select('webinar_' $webinar_id)) die('Вебинар не существует');
    }

    public function 
get($webinar_id$id 0$filter = [])
    {
        
$this->switch_db($webinar_id);
        
$where_clause = ($id == ? [] : ['id' => $id]) + $filter;
        
$query $this->db->get_where($this->table$where_clause);
        return (
$id == $query->result_array() : $query->row_array());
    }

    public function 
get_only_status($webinar_id$id 0$filter = [])
    {
        
$this->switch_db($webinar_id);
        
$where_clause = ($id == ? [] : ['id' => $id]) + $filter;
        
$query $this->db->select('id, status')->get_where($this->table$where_clause);
        return (
$id == $query->result_array() : $query->row_array());
    }

    public function 
add($webinar_id$data)
    {
        
$this->switch_db($webinar_id);
        
$data['status'] = 0;
        
$data['date'] = date('Y-m-d H:i:s');
        
$this->db->insert($this->table$data);
    }

    public function 
delete($webinar_id$id)
    {
        
$this->switch_db($webinar_id);
        
$this->db->delete($this->table, ['id' => $id]);
    }

    public function 
edit($webinar_id$id$data)
    {
        
$this->switch_db($webinar_id);
        
$data['date'] = date('Y-m-d H:i:s');
        
$this->db->update($this->table$data, ['id' => $id]);
    }

    public function 
update_status($webinar_id$data)
    {
        
$this->switch_db($webinar_id);
        if (
$data['status'] == 1)
        {
            
$tool $this->get($webinar_id$data['tool_id']);
            if (! empty(
$tool)) foreach (['banners','timers','polls','randoms','subscriptions'] as $table)
                
$this->db->update($table, ['status' => 0], ['block_id' => $tool['block_id'],'status' => 1]);
        }
        
$this->db->update($this->table, ['status' => $data['status']], ['id' => $data['tool_id']]);
    }


You see this $this->switch_db($webinar_id); at the beginning of each method? It is used because of a separate db for each webinar (conference) instance. The problem that i need to pass $webinar_id param every time, which i get in controller as part of a URL. Duplicating code looks a bit awkward. I thought about constructor - BUT i can't pass an argument to it. Undecided Any suggestions?

Please, don't argue against "multi-db" decision - it can't be undone and has several advantages (smaller databases, better backup strategy).
Reply

#2
I would probably add a set_webinar_id() method to set a protected property in the model to be used in place of the passed parameter and to call switch_db(). If the set_webinar_id() method returns $this, you could also use it inline, like this:

PHP Code:
$this->some_model->set_webinar_id($webinar_id)->update_status($data); 

Of course, if you're refactoring, this does mean that you would have to update all of your calls to all of the related models to set the webinar ID before calling any of the other methods.

Another alternative would be to set the $webinar_id as a property of the model itself, then load the model specific to the webinar. Whether that would be appropriate really depends on the specifics of your environment, and it seems pretty unlikely given the way your current model is setup.
Reply

#3
mwhitney, thank you for being responsive!

Quote:Another alternative would be to set the $webinar_id as a property of the model itself, then load the model specific to the webinar. Whether that would be appropriate really depends on the specifics of your environment, and it seems pretty unlikely given the way your current model is setup.

There would be hundreds, if not thousands, webinars. I can't load a specific model (with hard-coded database id) for each one.


Quote:I would probably add a set_webinar_id() method to set a protected property in the model to be used in place of the passed parameter and to call switch_db().

Could be fine, if i'd had several model's function calls in a controller. But typically i have "one URL call -> one model function call". So $this->some_model->set_webinar_id($webinar_id)->update_status($data) doesn't do much good compared to $this->some_model->update_status(webinar_id, $data) Anyway this reduces model's code and it is an interesting decision.
Reply

#4
The only other suggestion I could make would be to configure an autoloader (like Composer), create a model which doesn't extend CI_Model, and don't use CI's loader to load the model. Then you can pass the ID to the constructor.

The downside, of course, is that you have to make sure to load the database and you'll have to copy over the magic method from CI_Model (to access the CI instance via $this) if you want to limit the amount of code you'll need to change in your model.

The upside is that you could connect directly to the right database in the model's constructor, rather than switching databases.
Reply


Digg   Delicious   Reddit   Facebook   Twitter   StumbleUpon  


  Theme © 2014 iAndrew  
Powered By MyBB, © 2002-2020 MyBB Group.