2

I have a class that performs basic MySQL operations. This is all in PHP.

class dbTables {
    public $name;
    protected $fields = array(); // array of dbTableField objects
    public $result_sets = array();
    protected $primary_key; // filled using getFields() from __construct()


    public function __construct($table_name) {
        $this->name = $table_name;
        $this->getFields();
    }
    public function removeWhitespace($table_name) {
        $this->name = $table_name;
        $this->getFields();
    }
    // etc...
}

So all you need is the table name to create the object, and construct automatically gets a bunch of field data from INFORMATION_SCHEMA (names, data types, etc).

I also have a method for removing whitespace from a column It's currently located in another class, but I'm thinking of moving into the above dbTables class. You can see the method below. I don't think it's really necessary to read the method to answer my question, but it could be helpful.

protected function removeWhiteSpace($target_field,$abbrev_field,$from_size,$to_size,$options) {
    // optional conditions based on original character length and resulting character length.
    /* options: from_size = all/longer_than/shorter_than
    *   to_size = all/longer_than/shorter_than
    */
    $temp_table = $this->table_name;        
    $sql = "UPDATE `$temp_table` 
        SET `undo_sku` = `$target_field`, `$target_field` = REPLACE(`$target_field`,' ',''), `$abbrev_field` = 'w'";
    $where = array();
        if($options['from_size'] == 'longer_than') {
            $where[] = "`$target_field` > $from_size";
        } else if($options['from_size'] == 'shorter_than') {
            $where[] = "`$target_field` < $from_size";
        }
        if($options['to_size'] == 'longer_than') {
            $where[] = "LENGTH(REPLACE(`$target_field`,' ','')) > $to_size";
        } else if($options['from_size'] == 'shorter_than') {
            $where[] = "LENGTH(REPLACE(`$target_field`,' ','')) < $to_size";
        }               
        //WHERE `$target_field` = '' AND LENGTH(REPLACE(`vnd_sku`,' ','')) <= $string_length";
    $result = mysql_query($sql);
}

Now, if I do move it into dbTables, I will still want to use the functionality in the other class. Should I just instantiate a dbTables object whenever I need to use removeWhitespace? Or, could I give dbTables its own removeWhitespace method like this:

protected function removeWhiteSpace($target_field,$abbrev_field,$from_size,$to_size,$options) {
  /* options: from_size = all/longer_than/shorter_than
  * to_size = all/longer_than/shorter_than
  */
  $dbtable = new dbTables($this->table_name);
  $temp_table = $this->table_name;

  $result = $dbtable->removeWhiteSpace($target_field,$abbrev_field,$from_size,$to_size,$options);
  return $result;
}

The advantage I see is that now I have a way to limit instantiation of the dbTables object in various other classes. Methods that need to use removeWhiteSpace can use $this->removeWhiteSpace instead of instantiating a dbTables object. The class's own removeWhiteSpace method takes care of that.

I'm kind of new to OOP, though, so I'm just wondering if that is good practice or bad.

I also hope I'm asking this question in the right place. I'm pretty sure it's not a StackOverflow question.

2

I think the code smell here is that you have multiple classes that hold table names.

Good object-oriented design calls for a class that wraps a table (which itself holds reference to class that wraps database connection) and using that to refer to tables in all the other classes.

I suspect that's what you are aiming at with the dbTables class. So yes, move the method there, but replace the table_name field in the other class with reference to dbTables instance, so the wrapper just says $this->table->removeWhitespace(...).

  • Indeed I have run into problems due to my early use of table names. Are you recommending that I pass objects as parameters to other objects' methods, rather than strings/numbers? The "client" object could then use the parameter-object's methods to manipulate it or get data out of it? – Buttle Butkus Aug 24 '13 at 0:27
  • @ButtleButkus: You should be passing around objects. It gives you better checking, because it will fail quickly (on missing attribute) if you pass wrong type and it will hide the dereferencing inside the methods. Stop if it would make the code longer or more complicated though. – Jan Hudec Aug 26 '13 at 6:09
5

If you find yourself wanting to instantiate an object (e.g. dbTables) just to access one bit of very specific functionality in it, without actually using that object for what it is, then it's usually a code smell and a sign you need to portion up your code a little differently.

Why not factor out your removal of whitespace functionality, in a reusable and context-free a way as possible, into some sort of utility class? Then this can be accessed from all the places needing the functionality.

Alternatively, leave the function where it is, but make it static (class level), so that it can be called without using an instance of the class it's in.

  • Hi, sorry for the late reply. The removeWhitespace function in my example would go through a database table and remove the whitespace from every single column. I'm not sure if it would actually make sense for it to be in a utility class, which might have to work on plain strings, database tables, and database columns. What do you think? – Buttle Butkus Aug 24 '13 at 0:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.