Interface or Abstract Class?

PHP
1

I am trying to develop a DRY options class for things like site settings and addons but I am stuck as to whether the initial class should be abstract or an interface

	interface Options {
		static function init();
		static function get($key);
		static function set($key, $value);
	}
	
	class Settings implements Options {
		private static $_data = array();
	
		static function init() {
			$sql = 'SELECT name, value FROM settings';
			foreach ( db()->query($sql) as $r ) {
				self::$_data[$r['name']] = $r['value'];
			}
		}
	
		static function get( $key ) {
			if( self::exists($key) ) {
				return self::$_data[$key];
			}
		}
		
		static function set( $key, $value ) {
			if( self::exists($key) ) {
				$sql = "UPDATE settings 
					SET value = '".$value."' 
					WHERE name = '".$key."'";
			} else {
				$sql = "INSERT 
					INTO settings 
					VALUES('".$key."', '".$value."')
					LIMIT 1";			
			}
			db()->query($sql);
		}

		static function delete( $key ) {
			if( self::exists($key) ) {
				db()->query("DELETE 
					FROM settings 
					WHERE name = '".$key."'"
				);
			}
		}

		static function exists( $key ) {
			return array_key_exists($key, self::$_data);
		}	
	
	} Settings::init();

	class AddOns implements Options {
		private static $_data = array();
	
		static function init() {
			$sql = 'SELECT type, name FROM addons';
			foreach ( db()->query($sql) as $r ) {
				self::$_data[$r['type']][] = $r['name'];
			}
		}
	
		static function get( $type ) {
			if( array_key_exists($type, self::$_data) ) {
				return self::$_data[$type];
			}
		}
		
		static function set( $name, $type ) {
			if( !self::exists($name, $type) ) {
				db()->query("INSERT 
					INTO addons 
					VALUES('".$name."', '".$type."')
					LIMIT 1"
				);
			}
		}

		static function delete( $name, $type ) {
			if( self::exists($name, $type) ) {
				db()->query("DELETE 
					FROM addons 
					WHERE name = '".$name."' 
						AND type = '".$type."'"
				);
			}
		}

		static function exists( $name, $type ) {
			return array_search($name, self::get($type)) === FALSE
				? FALSE
				: TRUE; 
		}		
		
	} AddOns::init();
2

It depend on what you actually need.

You use abstract class if most of your extended classes will use at least parts of the ebstract’s code.
And you use interface if no shared code exists between the classes.
At least thats how i understand it.

BTW, registry pattern and static function in general are not the best way to do OOP, and it is a b*tch to write unittests for such code ( if you happen to use TDD ).

3

Don’t have an answer for your question but I’m wondering why do you use an “init” method why don’t you use the constructor? And why does it need to be a static class?

4

That I do understand, I hoped a second set of eyes could review if this can be done as an abstract class or keep it as a interface based on the code above. I note code that seems to be similar that could be part of an abstract class…

The init method would call the static class right away like a construct when initializing a class so the information can be loaded up right away for future use.

Static is bad I take it? :eek:
The idea was to be able to use the methods without having to initialize the class and re-querying the database.

What would be another option? I am currently looking over a similar function in the Zend Framework atm.

5

Assuming that your table structure is always going to be laid out the same way, this is how I would write it:

<?php

abstract class Options
{    
    protected $data;
    protected $tableName;
    protected $keyFieldName;
    protected $valueFieldName;
    
    
    abstract protected function defineTableStructure();
    
    
    public function __construct()
    {
        $this->data = array();
        $this->defineTableStructure();
        
        $sql = 'SELECT '.$this->keyFieldName.', '.$this->valueFieldName.' FROM '.$this->tableName;
        foreach ( db()->query($sql) as $r ) {
            self::$_data[$r[$this->keyFieldName]] = $r[$this->valueFieldName];
        }
    }
    
    public function get( $key )
    {
        if( $this->exists($key) ) {
            return $this->data[$key];
        }
    }
    

    
    public function set( $key, $value )
    {
        if( $this->exists($key) ) {
            
            $sql = "UPDATE ".$this->tableName." 
                    SET ".$this->valueFieldName." = '".$value."' 
                    WHERE ".$this->keyFieldName." = '".$key."'";
        
        } else {
        
            $sql = "INSERT 
                    INTO ".$this->tableName."
                    VALUES('".$key."', '".$value."')
                    LIMIT 1";            
        }
        db()->query($sql);
    }
    

    public function delete( $key )
    {
        if( $this->exists($key) ) {
            
            db()->query("
                DELETE 
                FROM ".$this->tableName." 
                WHERE ".$this->keyFieldName." = '".$key."'"
            );
        }
    }
    

    protected function exists( $key )
    {
        return array_key_exists($key, $this->data);
    }    
    
}

<?php

class Settings extends Options
{

    protected function defineTableStructure()
    {
        $this->tableName        = 'settings';
        $this->keyFieldName        = 'name';
        $this->valueFieldName    = 'value';
    }
}

<?php
class AddOns extends Options
{
    protected function defineTableStructure()
    {
        $this->tableName        = 'addons';
        $this->keyFieldName        = 'name';
        $this->valueFieldName    = 'type';
    }
}
6

Yeah, that is what I was thinking but the Addons would be an array type->array(), whereas the settings would be a straight key->value. The tables are laid out exactly the same, just named differently, again basically a key->value

7

Ah missed that. Although you could modify the abstract class to handle both cases easily enough.

8

Hi…

Could you repost the code stripping out everything you are not absolutely sure you need?

yours, Marcus

9

I don’t think I am following you Marcus.

I can post the barebones of what I think I may have needed:


    class Options {
    	private $_data = array();
    	function __construct();
    	function get( $key );
    	function set( $key, $value );
    	function delete( $key, $value = null );
    	function exists( $key, $value = null );
    }

But as above I was thinking as having it statically so I can do this:


Setting::get('website_title');
Setting::set('website_title', 'MySite');
Setting::delete('website_title');

foreach( AddOns::get('plugin') as $plugin ) {
	// load plugins
}
AddOns::set('plugin', 'Hello World');
AddOns::set('plugin', 'SpamBlocker');

if( isset($_GET['active-template']) ) {
	AddOns::delete('template', AddOns::get('template')[0]); // not implemented yet in PHP
	AddOns::set('template', $_GET['active-template']);
}
10

Hi…

You got it exactly. Isn’t this just stdClass?

yours, Marcus

11

A few things

  1. You use the class as if its methods and fields are static; they are not. While PHP may be fine with this when your error_reporting level is low, it is technically incorrect and higher error_reporting levels (like E_ALL) don’t even allow it. You should change the methods and fields to static and use self::$_data instead of $this->_data

  2. Your set function is utterly confusing. For example:


AddOns::set('plugin', 'Hello World');
AddOns::set('plugin', 'SpamBlocker');

Does this first set the ‘plugin’ key to ‘Hello world’ and then replace it with ‘SpamBlocker’, or does it first add ‘Hello world’ and then ‘Spam blocker’ so the key ‘plugin’ actually has two values (or rather an array with two values).
If it’s the latter I think the name ‘set’ is inappropriate since I would expect a function with that name to override a previous setting rather than adding to it.
Also, when you use it to add things it doesn’t make sense for variables like ‘website_title’ of which it doesn’t have sense to have multiple.
I would suggest you make a distinction between scalar noparse and vector (::add)[/noparse] types in the class to make it more clear what’s going on.