CODEWITHSUNDEEP
phpoopinheritancestatic
Dan Lugg
Dan Lugg

Reputation: 20592

Possible poor design; forcing static definition in inheriting classes in PHP

I think I may have hit a wall due to poor design decisions.

I'm working on a class hierarchy, with an abstract base from which a few classes extend. Without getting into the "nitty-gritty" of why, the function of instances of these classes require that a "context" object be available -- the currently operating object.

abstract class AbstractBase{

    protected static $_context;

    protected $_parent;

    public static function get_context(){
        return static::$_context;
    }

    protected static function set_context(self $context, &$previous = null){
        $previous = static::get_context();
        static::$_context = $context;
    }

    public function __construct(){
        $this->_parent = static::get_context();
    }

    public function do_something($callback){
        static::set_context($this, $previous);
        $callback();
        static::set_context($previous);
    }

}

class ConcreteOne extends AbstractBase{

}

class ConcreteTwo extends AbstractBase{

}

This works fine, except that ConcreteOne and ConcreteTwo need to keep track of their own context -- the current definition will cause any context change from any inheriting class to overwrite AbstractBase::$_context. This change is reasonably easy to implement:

class ConcreteOne extends AbstractBase{

    protected static $_context;

}

class ConcreteTwo extends AbstractBase{

    protected static $_context;

}

Now the concrete implementations will manage their own contexts. This however presents a bit of an issue; any client classes extending the base class will have the requirement of a static member named $_context.

This stinks of bad design to me, but then again, my nose isn't the sharpest yet. I'm wondering if I've moseyed down a bad path here, and most importantly, if I should continue down said path or abort and change it up.

So, should I forge ahead, or can someone suggest a better solution to managing a "static" context across instances?

Note: I've considered passing the $context object as an argument to the $callback, however since ConcreteOne callbacks may create instances of, and call, ConcreteTwo objects (and vice versa, and any other inheriting class) that could result in a great number of context objects needing to be passed around at a given time -- I don't think that's a solution.

Upvotes: 3

Views: 197

Answers (2)

Jared Farrish
Jared Farrish

Reputation: 49188

I'm going to go ahead and post this as an answer, since I believe this is what you are working towards in the end.

The problem I believe you're spotting is that you have the abstract class property $_context, which is setup to store a single context, yet to make it work right, you're really needing a reference within/by the actual class itself. So the $_context property, in your example, is actually a reference to the current object's context, NOT a single context for all instantiated classes that extend your AbstractBase.

This, of course, doesn't make sense. What I would suggest is a tweak to your current static $_context and use it as a store to save and retrieve your references, returning a unique ID when the context is set and putting that ID into a $this->context_id so the object can continue to get it's context by reference:

<?php

abstract class AbstractBase {
    protected static $_context;
    private $context_id;

    public static function setContext($context) {
        $context_id = uniqid(php_uname('n'), true);
        self::$_context[$context_id] = $context;
        return $context_id;
    }

    public function getContext() {
        return self::$_context[$this->context_id];
    }

    public function myPrint() {
        print_r($this);
        print_r(self::$_context);
    }
}

class test1 extends AbstractBase {
    public function __construct($context){
        $this->context_id = self::setContext($context);
    }
}

class test2 extends AbstractBase {
    public function __construct($context){
        $this->context_id = self::setContext($context);
    }
}

$test1 = new test1('this stuff');
$test2 = new test2('other stuff');

$test1->myPrint();
$test2->myPrint();

?>

http://codepad.org/1FlYkTN1

Upvotes: 1

matthijs koevoets
matthijs koevoets

Reputation: 386

This does look a bit forced. I think a 'regular' base class that has static methods - or having a static object would be a better strategy.

Upvotes: 0

Related Questions