SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 27
  1. #1
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Same method used in several classes

    Say I have some methods which must exist in every class, I write an interface

    PHP Code:
    interface Common {
       private function 
    load() {}
       private function 
    validate ($var) {}
       public function 
    save($var) {}
       public function 
    get($key '') {}

    But then say, that I always want one of the functions to do exactly the same thing:

    PHP Code:
    public function get($key '') {
        if (empty(
    $this->value)) {
            
    $this->load();
        }
        if(!empty(
    $key)) {
             if(isset(
    $this->value[$key])) {
                 return(
    $this->value[$key]);
             }
             else {
                  return 
    '';
             }
        }
        return 
    $this->value;

    Used in an example class:

    PHP Code:
    class Invoice implements Common{
        
    // I won't implement the validate() and save() mehod in this example
        
    private $value;
        public function 
    __construct($id) {
            
    $this->value['id'] = intval($id);
        }
        private 
    load() {
             
    // database logic gets an assoc array in row
             
    $this->value['name'] = $row['name'];
        }
        
    // I want to use a general get method

    How would I go about having the object working like this without having to rewrite the get() method. Do I have to extend to a common class for all the functions?

    PHP Code:
    define('INVOICE'1);
    $invoice = new Invoice(INVOICE);
    echo 
    $invoice->get('id'); 
    Please advice - also if you think the method sucks, just tell how to go about it instead

  2. #2
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    An approach could be to return an object with the values. For instance something like the following:

    PHP Code:
    class Values {

        private 
    $value;

        
    /**
         * get()
         * @param  string $key
         * @return string $value
         */
        
    public function get($key '') {
            if(!empty(
    $key)) {
                if(isset(
    $this->value[$key])) {
                    return(
    $this->value[$key]);
                }
                else {
                    return 
    '';
                }
            }
            return 
    $this->value;
        }

        
    /**
         * setString()
         * @param  string $key
         * @return string $value
         */
        
    public function setString($key$value) {
            
    $this->value[$key] = $value;
            return 
    true;
        }

        
    /**
         * addArray()
         * @param  string $key
         * @return string $value
         */
        
    public function addArray($array) {
            if (!
    is_array($array) OR empty($array)) {
                return 
    false;
            }
            return 
    $this->value array_merge($array$this->value);

        }

    And then in the example having:

    PHP Code:
    class Invoice implements Common {
        private 
    $value_object;
        public function 
    __construct($value_object) {
             
    $this->value_object $value_object;
        }
        private 
    load() {
            
    // database logic returning row
            
    $this->value_object->setString('name'$row['name']);
        }
        public 
    get() {
            return 
    $this->value_object;
        }

    And the usage

    PHP Code:
    define('INVOICE'1);
    $value = new Values;
    $value->setString('id'INVOICE);
    $invoice = new Invoice($value);
    echo 
    $value->get('name'); 

  3. #3
    SitePoint Zealot
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    170
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lsolesen View Post
    How would I go about having the object working like this without having to rewrite the get() method. Do I have to extend to a common class for all the functions?
    If you want to force classes to implement methods (provide a strict contract) and inherit common methods you could use an abstract class. Matt Zandstra discussed the concept in-depth at zend.com.

  4. #4
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    So something down the lines of:

    PHP Code:
    interface iCommon {
        
    // with the previously mentioned methods
    }

    abstract class 
    Common implements iCommon {
        public function 
    get($key '') {
            
    // the code from before
        
    }
    }

    class 
    Invoice extends Common {
        
    // making sure that the methods from iCommon are present
        // able to utilize the common get()-method from Common


  5. #5
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    To me it sounds like there is another class begging to be created somewhere, instances of which can be passed into objects that need them. An interface is not a solution here (the name "Common" should be a give away...its pretty meaningless) and whilst an abstract class would certainly be an option, I'd look towards a solution that uses composition over inheritance. If all the objects that use these "common" methods are indeed variations of the same thing then inheritance might be acceptable but otherwise I'd avoid it.

  6. #6
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Luke Redpath --> would that mean something like the second solution in post #2 i suggested, because that was inspired by my understanding of the composite pattern. However I am not certain how to apply it correctly.

    What I want to do is having the interface for all my classes be similar. If I have an Invoice I will be able to do something like this:

    $invoice = new Invoice(2);
    $invoice->get('id'); //2

    $customer = new Customer(2);
    $customer->get('id'); // 2

    The get method was implemented through the interface to make sure I had it present, and it was written in the abstract class so I wouldn't have to rewrite it. I think myself that it is not completely right, but I fail to see how to do it otherwise, can you explain how you would do it?

  7. #7
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Watch out - Composition is a general technique of re-use. It should not be confused with the composite pattern.

    You generally have two ways to re-use objects in OO. Through implementation inheritance (aka. inheritance) or through composition (sometimes called aggregation). There are pros and cons of both strategies, but I think it's safe to say that inheritance is often over- and misused. In this particular case, I think it may apply, but it is often better to use composition.

  8. #8
    SitePoint Wizard stereofrog's Avatar
    Join Date
    Apr 2004
    Location
    germany
    Posts
    4,324
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lsolesen View Post

    How would I go about having the object working like this without having to rewrite the get() method.
    Ruby people are used to use Mixins for this. Mixin is essentialy a collection of implementations not bound to a particular class. Any class that provides necessary callbacks can utilize a Mixin.
    Code:
    module Logger
    	def log
    		do_something_with log_data
    	end
    end
    
    class Whatever
    	include Logger
    	
    	def log_data
    		"log of me"
    	end
    end
    
    Whatever.new.log
    In php there's no automated Mixins, therefore we have to write the explicit delegation code.

    PHP Code:
    class Logger
    {
        function 
    __construct($that) {
            
    $this->that $that;
        }
        function 
    log() {
            
    $this->do_something_with(
                    
    $this->that->log_data());
        }
    }

    class 
    Whatever
    {
        function 
    __construct() {
            
    $this->logger = new Logger($this);
        }
        function 
    log_data() {
            return 
    "log of me";
        }
    }

    $w = new Whatever;
    $w->logger->log(); 
    Practically, this is what Luke said.

  9. #9
    SitePoint Addict
    Join Date
    Nov 2005
    Location
    Germany
    Posts
    235
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I do the following: As my domain objects tend to be pretty similar I use a class DomainObject with common behaviour to be extended by the real domain classes. (pull up method)
    PHP Code:
    class DomainObject {
      public function 
    __construct($id, ...) {...}
      public static function 
    isValid($id, ...) {...}
      public static function 
    create(...) {...}
      public static function 
    delete($id, ...) {...}
      public function 
    simpleSet($column$value) {...}
    }

    class 
    Employee extends DomainObject {
      public function 
    __construct($id, ...) {
        
    parent::__construct($id, ...);
        ...
      }

    Since you seem to have absolutely the same behavior you wouldn't even need to override and call the parent.

  10. #10
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    FriB - this is actually the kind of bad code smell I'm talking about. If you find you have a number of methods that you use across multiple classes, then it usually means there is a "hidden" object trying to fight its way out. You should try and refactor your code to extract this functionality (aka Extract Class refactoring) into its own object and then pass this object around to the ones that need that functionality.

    Looking at your example above -- and I'm making some assumptions as I don't really know anything about your domain (and your example is likely just generic) -- the create and delete functions look like they belong to some kind of object of their own. isValid sounds like it belongs to some kind of validator object which could be passed around.

    You may still have a case for a base object in an inheritance tree (for instance Ruby has an Object class which *all* objects inherit from) but the behaviour in that base class should *only* represent behaviour *inherent* to that object (and its subclasses), not behaviour *used* by its subclasses. (an example: you may have the need for some kind of data access layer through out many of your domain model objects but even though your domain objects uses this dao, the dao functionality isn't *inherent* to your domain model objects - therefore it makes sense to pass around a dao object to the objects that need it rather than making its functionality available through the base class).

    Classes should be focussed on a particular subset of behaviour rather than trying to lump all kinds of methods into a class (a god class) and the same principle applies to ancestors in an inheritance tree too.

    Hope that makes sense.

  11. #11
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Luke Redpath --> it makes sense, I think. The question is how to do it exacty? How would you solve the situation in comment #6 which I will clarify now.

    I have a lot of different objects, say Invoice, Contact, Intranet. All classes represent one thing, and every class should be able to return something about the class to me. What I have done so far is to load variables into an array $this->value and then use a method from a parent class (get) to return it like this $invoice->get('id').

    But as I understand this thread, this design is not good, as the parent class is a "god" class. But how will I do it instead? Could a solution be as in comment #2? Or is the approach totally off? It would be helpful to know how you would solve the initial class design as shown i comment #1.

  12. #12
    SitePoint Guru silver trophy Luke Redpath's Avatar
    Join Date
    Mar 2003
    Location
    London
    Posts
    794
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    If those three objects really are the same kind of "thing" then inheritance may be acceptable. I'm just not sure that they really are. In Ruby I'd certainly be looking at a mixin but unfortunately you don't have that luxury.

    What it seems you are trying to do here is create some kind of generic interface for getting and setting variables on any particular object, if I'm understanding you correctly. In this case, perhaps some kind of base Object class would make sense (with functionality for getting/setting attributes which would be suitable behaviour for an Object class). However...one thing puzzles me. Why do you need this generic get/set functionality? Whats wrong with doing $invoice->id() - why $invoice->get('id')? A generic getter/setter on all objects seems like unnecessary violation of encapsulation. Just expose the data you need through dedicated methods instead.

  13. #13
    SitePoint Addict
    Join Date
    Nov 2005
    Location
    Germany
    Posts
    235
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Luke Redpath View Post
    FriB - this is actually the kind of bad code smell I'm talking about.
    I knew that, I wanted to throw in a simple solution to the problem anyway as this doesn't seem that bad.
    Looking at your example above -- and I'm making some assumptions as I don't really know anything about your domain (and your example is likely just generic) -- the create and delete functions look like they belong to some kind of object of their own. isValid sounds like it belongs to some kind of validator object which could be passed around.
    In a simple domain and with only a couple of big domain classes, by big I mean something like maybe 15 (short and very ungodly) public methods (so not that big at all), UnattendedTest::valid($id, ...) seems ok for me. When the class becomes more complex I agree that I would have to split behaviour into more appropriate classes.

    You may still have a case for a base object in an inheritance tree (for instance Ruby has an Object class which *all* objects inherit from) but the behaviour in that base class should *only* represent behaviour *inherent* to that object (and its subclasses), not behaviour *used* by its subclasses.
    This seems valid for simpleSet() and thanks, I will think about pulling this one out at the next iteration.
    Hope that makes sense.
    Yes indeed, thanks for the comment.

  14. #14
    SitePoint Guru BerislavLopac's Avatar
    Join Date
    Sep 2004
    Location
    Zagreb, Croatia
    Posts
    830
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Luke Redpath View Post
    In Ruby I'd certainly be looking at a mixin but unfortunately you don't have that luxury.
    Actually, PHP has the next best thing -- global functions. I know this is not ideal, especially with the PHP's lack of namespaces, but I can easily envision something along the lines of:

    PHP Code:

    interface MyInterface
    {
        public function 
    foo();
        public function 
    bar();
    }

    class 
    FooClass implements MyInterface
    {
        public function 
    foo()
        {
            
    // some code
        
    }

        public function 
    bar()
        {
            
    do_basic_bar_behavior();
        }
    }

    class 
    BarClass implements MyInterface
    {
        public function 
    foo()
        {
            
    // some other code
        
    }

        public function 
    bar()
        {
            
    do_basic_bar_behavior();
        }
    }

    function 
    do_basic_bar_behavior()
    {
        
    // some generic code

    In my opinion, PHP's mix of object and procedural style orientation is a blessing in disguise. Even though it may encourage bad code, with good planning it gives quite a lot of power.

  15. #15
    SitePoint Guru dbevfat's Avatar
    Join Date
    Dec 2004
    Location
    ljubljana, slovenia
    Posts
    684
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    How about a simple static delegation?

    PHP Code:
    class CommonImplementation
    {
      function 
    doSomething()
      {
        
    // when called statically, $this refers to the original caller
        
    echo $this->value 7;
      }
    }

    class 
    MyClass
    {
      function 
    doSomething()
      {
        
    CommonImplementation::doSomething();
      }
    }

    $c = new MyClass();
    $c->value 4;
    $c->doSomething(); // 28 
    You'd still have to write the delegation code in every class, but it's literally a one-liner without implementation.

    I also found a few articles on mixins in PHP, here is an older one and a newer (php5 ready) one.

  16. #16
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I want to do it as properly as possible, so I don't want global variables or global functions, as they probably will be a nightmare to test (and I want to make it testable).

  17. #17
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Luke Redpath View Post
    However...one thing puzzles me. Why do you need this generic get/set functionality? Whats wrong with doing $invoice->id() - why $invoice->get('id')? A generic getter/setter on all objects seems like unnecessary violation of encapsulation. Just expose the data you need through dedicated methods instead.
    Well, I don't want to write a get-method for every single value which should be available, so I wanted to do a generic version, so everything in my $this->value scope is available through the get-method. In invoice I have 15 variables which should be public and in Contact about the same. That would be a lot of extra methods to clutter up the classes. And that is only those two classes. I have 15 classes more which all needs to expose their variables so it can be shown on a internet page for instance.

  18. #18
    SitePoint Guru BerislavLopac's Avatar
    Join Date
    Sep 2004
    Location
    Zagreb, Croatia
    Posts
    830
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Isolesen, "properly" is a stretchy word. If by that you mean "proper OOP", keep in mind that there is no such thing (except perhaps in Smalltalk), and that OOP is always "tainted" by the specific language's implementation. If, on the other hand, you mean "proper usage of PHP", then there's nothing wrong with smartly using procedural code (actually, you can't avoid it) to complement your OOP.

  19. #19
    SitePoint Zealot
    Join Date
    Oct 2003
    Location
    Denmark
    Posts
    129
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    BerislavLopac --> yeah, I know that there is no true solution. I am just looking for a solution which will stand tough. I know that procedural and oop code can be mixed, but I don't see how it would apply in this case? Could you explain how to solve the design problem with code examples? I tend to think that code is more persuading than words

  20. #20
    SitePoint Guru BerislavLopac's Avatar
    Join Date
    Sep 2004
    Location
    Zagreb, Croatia
    Posts
    830
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Check my post above.

  21. #21
    SitePoint Zealot
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    170
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lsolesen View Post
    Well, I don't want to write a get-method for every single value which should be available
    You don't have to if you implement method overloading with __call(). Just be cautious with objects, because magic methods don't return by reference.

  22. #22
    SitePoint Enthusiast
    Join Date
    Jul 2004
    Location
    Finland
    Posts
    73
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by michel View Post
    ...
    Just be cautious with objects, because magic methods don't return by reference.
    Yes they do.

    PHP Code:
    class TestClass{    
        public 
    $test;
        function 
    __call($method,$args){
            if (
    $method == 'getTest'){
                return 
    $this->test;    
            }
        }
    }
    $a = new TestClass();
    $a->test = new stdClass();
    if (
    $a->test === $a->getTest()){
        echo 
    "ok";    


  23. #23
    SitePoint Zealot
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    170
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by JaskaS View Post
    Yes they do.
    Yep, too quick with that, my bad. I was working on an old box with a pre 5.1 version. Apparently an old bug.

  24. #24
    SitePoint Wizard stereofrog's Avatar
    Join Date
    Apr 2004
    Location
    germany
    Posts
    4,324
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    __call doesn't return by reference, nor does this the above code. The thing is, in php5 you don't need to reference objects, because objects are now pointers.

  25. #25
    SitePoint Enthusiast
    Join Date
    Jul 2004
    Location
    Finland
    Posts
    73
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by stereofrog View Post
    __call doesn't return by reference, nor does this the above code. The thing is, in php5 you don't need to reference objects, because objects are now pointers.
    Well technically they are not references or pointers, if compared to c pointers and references or Java references. But let's not start this debate again :P


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •