SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 32
  1. #1
    SitePoint Wizard
    Join Date
    Aug 2004
    Location
    California
    Posts
    1,672
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Pseudo-OOP Code?

    I happened to click to PHPBuilder today and saw that a PEAR Validation class was recently released, so I clicked over to take a look. Now I don't just want to single out this code, but I was very suprised that code like this was actively maintained in PEAR. The class contains validation function for numbers, email addresses, strings, dates, and URLs. The code is not bad and they have unit tests -- that's good. The thing is, it is not really a class at all. It is just a bunch of stand-alone functions sort of namespaced into a class. There are no class properties. There are some "private" functions, but only one is called more than once so the others could be in-lined. It really should be a bunch of helper functions and should be packaged as such.

    This is strange because Rule based OOP Validator designs are very common and a quick Google would find Java, .NET, Python, etc. examples of current Validation classes. I know because I have found them when I have looked for ideas for Validation.

    I guess I'm just wondering how prevelant is this kind of procedural pseudo-OOP code is in PHP?
    Christopher

  2. #2
    SitePoint Addict
    Join Date
    Nov 2005
    Posts
    327
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I put a bunch of static methods in my Utility class that could as easily have been standalone functions. I do it mainly so __autoload() can find the code instead of me having to use include() or require() everytime I use it - which I do in every script.

  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 arborint
    I guess I'm just wondering how prevelant is this kind of procedural pseudo-OOP code is in PHP?
    It might be common for folks who're in 'transition' from procedural to OOP (regarding app design). I used to think this was a convenient way to namespace functions, but I found that it discourages the use of small, dedicated classes. They quickly became 'catch-alls', with no clear goal.
    I noticed a similar trend with some Java interfaces, where *Utils classes became deprecated and methods were moved/refactored to dedicated classes.

  4. #4
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    let's not be retarded here. There is more than 1 type of class. Static classes have their place too.

  5. #5
    SitePoint Zealot
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    170
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by dreamscape
    let's not be retarded here.
    Let's try to learn something instead. When does a static class start to smell bad?

  6. #6
    SitePoint Guru dbevfat's Avatar
    Join Date
    Dec 2004
    Location
    ljubljana, slovenia
    Posts
    684
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by michel
    Let's try to learn something instead. When does a static class start to smell bad?
    When you try to write tests for it.

  7. #7
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    You shouldn't test static classes? Well that's a bit of an odd notion.

  8. #8
    SitePoint Evangelist Scheisskopf's Avatar
    Join Date
    Nov 2004
    Location
    Southampton, UK
    Posts
    537
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    When does a static class start to smell bad?
    When it's a PE class and you forgot to open the windows

  9. #9
    SitePoint Wizard dreamscape's Avatar
    Join Date
    Aug 2005
    Posts
    1,080
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by michel
    When does a static class start to smell bad?
    And what exactly smells bad about a collection of utilities? A string utility? or a number utility? or a math utility? or a validation utility?

    Maybe your nose is the problem. Ever think of that. [for the record, yes I have thought that it could also be the code that is the problem, but I'm not stuck in that line of mentality]

    You know just because you might do someone differently than someone else does it, doesn't mean that the other person's code smells bad. Maybe it does to you, but then just plug your nose and walk away.

  10. #10
    SitePoint Addict
    Join Date
    Nov 2005
    Posts
    327
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by michel
    It might be common for folks who're in 'transition' from procedural to OOP (regarding app design). I used to think this was a convenient way to namespace functions, but I found that it discourages the use of small, dedicated classes. They quickly became 'catch-alls', with no clear goal.
    I noticed a similar trend with some Java interfaces, where *Utils classes became deprecated and methods were moved/refactored to dedicated classes.
    I do consider my Utility class a sort of catch-all for functions that don't fit elsewhere or warrant a whole separate class. I've also considered the possibilty that it may be refactored into several smaller classes at some point in the future. I just don't see a need for that at present. As I said, the important thing to me is that the class gets loaded by my __autoload() function when I call one of its methods, without me having to explicitly require('/some/long/path/that/I/got/sick/of/typing') in each and every script.

  11. #11
    SitePoint Zealot
    Join Date
    Jul 2004
    Location
    The Netherlands
    Posts
    170
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by dreamscape
    You know just because you might do someone differently than someone else does it, doesn't mean that the other person's code smells bad. Maybe it does to you, but then just plug your nose and walk away.
    You see right through me Where on earth did you get that? I'm trying to learn something here. Being pedantic gets us nowhere.

  12. #12
    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)
    Quote Originally Posted by dreamscape
    And what exactly smells bad about a collection of utilities? A string utility? or a number utility? or a math utility? or a validation utility?
    The moment you label something utility, you know you're lossing terrain in the battle against the forces of chaos. Like dotty, I often have some functions/classes which I put in a utility-scope. I then end up removing theese classes, or move merge them/move them into some other package later on.

    Quote Originally Posted by dreamscape
    You shouldn't test static classes? Well that's a bit of an odd notion.
    I think he meant that it was harder to do so.
    Anyway - static methods/classes will never go away completely, but in an object-oriented codebase, they are noise, since they operate in the global scope. Static methods are often seen in conjunction with use of static function-variables, or globals, which are notoriously hard to test properly. As long as the static method only relies on passed-in variables, you should be fine though.

    In the past I used to write quite a lot of static classes - I basically used the class-struct as a namespace for my functions in an overall procedural-style code. I personally prefer oo over procedural theese days, but I acknoledge that it's ultimately a subjective choice. If you do work with procedural style, you may well want to use classes as namespaces (or rather package is the more correct term).

  13. #13
    SitePoint Addict
    Join Date
    Nov 2005
    Posts
    327
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by kyberfabrikken
    Static methods are often seen in conjunction with use of static function-variables, or globals, which are notoriously hard to test properly. As long as the static method only relies on passed-in variables, you should be fine though.
    This is in fact one of the criteria for inclusion in my Utility class: a function must use only passed-in variables. Admittedly there are a couple of methods in it that require a constant to be defined in the configuration file. In those situations, the method definition provides a "most likely scenario" default value if the constant isn't defined.

  14. #14
    SitePoint Guru dbevfat's Avatar
    Join Date
    Dec 2004
    Location
    ljubljana, slovenia
    Posts
    684
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by kyberfabrikken
    I think he meant that it was harder to do so.
    That was exactly what I meant

    ... As long as the static method only relies on passed-in variables, you should be fine though.
    Well, it's still harder to mock static classes (= classes with static methods and properties). In a way, it is similar to testing a singleton.

  15. #15
    SitePoint Wizard
    Join Date
    Aug 2004
    Location
    California
    Posts
    1,672
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by dreamscape
    let's not be retarded here. There is more than 1 type of class. Static classes have their place too.
    I agree that there is more than one kind of class. My point was not necessarily that you shouldn't do static classes, but in PHP these functions would so obviously be better off as individual files with one function in each OR to be implemented as a modern, rule based, OO Validator that is completely extensible.

    This code is in active development in The PHP Class Archive -- and it's really a procedural library. I'm just wondering if the many "PHP can't really do OOP" or "I don't see the point of using classes" comments come from people looking at things like this?
    Christopher

  16. #16
    Web-coding NINJA! silver trophy beetle's Avatar
    Join Date
    Jul 2002
    Location
    Dallas, TX
    Posts
    2,900
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by arborint
    I guess I'm just wondering how prevelant is this kind of procedural pseudo-OOP code is in PHP?
    It's more common in PEAR than it should be
    beetle a.k.a. Peter Bailey
    blogs: php | prophp | security | design | zen | software
    refs: dhtml | gecko | prototype | phpdocs | unicode | charsets
    tools: ide | ftp | regex | ffdev




  17. #17
    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)
    Quote Originally Posted by dbevfat
    Well, it's still harder to mock static classes (= classes with static methods and properties). In a way, it is similar to testing a singleton.
    Good point.
    A Singleton is a static method, so they're bound to be similar.

  18. #18
    SitePoint Addict
    Join Date
    Nov 2005
    Posts
    327
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by beetle
    It's more common in PEAR than it should be
    I've seen it in a few packages myself without having looked at PEAR that much. It doesn't bother me. As far as I'm concerned, a third party script is a third party script. I put the files I need under my include directory and leave the rest. If I want to wrap them in an adapter to gain some real or imagined benefit, then I do so. I'd be the last one to tell someone else what their coding standards and practices should be. I just consider myself lucky that someone was generous enough to share their work so I don't have to reinvent the wheel.

  19. #19
    SitePoint Addict
    Join Date
    May 2003
    Location
    Calgary, Alberta, Canada
    Posts
    275
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by arborint
    I guess I'm just wondering how prevelant is this kind of procedural pseudo-OOP code is in PHP?
    I dont think this type of code is inherent to PHP but is just as prevelant in Java, ruby, C# etc. The line between what is OO and procedural is somewhat blurred. Like you say, if I have a group of functions and then I wrap add "class Name {" before the functions and a " } " after them do I now have object oriented code?

    There are a couple of reasons for code like this. 1) The code is very immature and hasnt been through the trenches yet, not refactored or used alot. 2) The code has been through the trenches but the developer hasnt and is inexperienced.

    For me the code smells to look out for are:
    - Private methods (Who's responsibility is this method? Not easily testable.)
    - Singletons (Is this really a singleton? I dont really need global access do I?)
    - Static methods (Do I need polymorphism? Why is this static? Why doesnt this object have any state?)
    - methods that return anything (Shouldnt I tell this method to do something instead of asking it for something?)
    - if statements (Cant I use polymorphism here?)
    - switch statements (Cant I use polymorphism here?)
    - instance of / is_a (Cant I use polymorphism here?)
    - nested if statements (Make a new private method, this is easier to read. See private methods )

    These are the things that I have been concious of recently.

    I tend to get classes with alot of private methods, usually this indicates I need a new class and my code is better for it.

    I never use static methods anymore. I never use singleton, one exception I have made is in a performance situation where I would load a large amount of data to the class through a file on each instance. Singleton solved this by only loading the data once for each instance. (Very specific circumstances)

  20. #20
    SitePoint Guru
    Join Date
    May 2005
    Location
    Finland
    Posts
    608
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Brenden Vickery
    - Private methods ... Not easily testable.
    This is a misleading statement. The fact that the private method in question works is evident in the fact that the public functions that it interfaces with do work. That is, the private method itself need not be tested.
    - instance of / is_a (Cant I use polymorphism here?)
    Err..?

  21. #21
    Web-coding NINJA! silver trophy beetle's Avatar
    Join Date
    Jul 2002
    Location
    Dallas, TX
    Posts
    2,900
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by a.dotty.dot
    I've seen it in a few packages myself without having looked at PEAR that much. It doesn't bother me. As far as I'm concerned, a third party script is a third party script. I put the files I need under my include directory and leave the rest. If I want to wrap them in an adapter to gain some real or imagined benefit, then I do so. I'd be the last one to tell someone else what their coding standards and practices should be. I just consider myself lucky that someone was generous enough to share their work so I don't have to reinvent the wheel.
    Ya, but PEAR is attempting to be a PHP framework, not just a collection of "third party scripts".

    It succeeds moderately at both, being better at the latter. What I cite above, however, is a problem for the former.

    Quote Originally Posted by Brenden Vickery
    - methods that return anything (Shouldnt I tell this method to do something instead of asking it for something?)
    What? Why does this raise a flag for you? One of the basic powers of delegating processes to functions (et methods) is the ability to receive a return value. Be it a success state, value transformation, or something else altogether like a Factory.
    beetle a.k.a. Peter Bailey
    blogs: php | prophp | security | design | zen | software
    refs: dhtml | gecko | prototype | phpdocs | unicode | charsets
    tools: ide | ftp | regex | ffdev




  22. #22
    SitePoint Addict
    Join Date
    May 2003
    Location
    Calgary, Alberta, Canada
    Posts
    275
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Ezku
    This is a misleading statement. The fact that the private method in question works is evident in the fact that the public functions that it interfaces with do work. That is, the private method itself need not be tested.
    Err..?
    Misleading wasnt the intent. Assuming the public method is tested properly, yes the private method is tested aswell. What Im trying to get at is that sometimes having a private method or more like many private methods is sometimes an indication your class is doing too much. Take the code below for example (possibly too simple example):
    PHP Code:
    class City {
        private 
    $citizens;
        private 
    $altitude;
        private 
    $name;
        public function 
    getPrivateCitizensNamedJohn() {
            
    $result = array();
            foreach(
    $this->citizens as $citizen) {
                if(
    $this->isPrivateCitizenNamedJohn($citizen)) {
                    
    $result[] = $citizen;
                }
            }
            return 
    $result;
        }
        private function 
    isPrivateCitizenNamedJohn($citizen) {
            return (
    $citizen->isPrivate() && $citizen->getName() == 'John');
        }

    While the private method is testable through the public method there is a problem. The isPrivateCitizenNamedJohn method is the responsibility of the Citizen class not the City class. Move the method to the Citizen class and the method is now public, more easily testable and has the right responsibility in the right place.
    Private methods are just an indication to me that I *may* have a problem and should take a closer look. They are certainly not something to be avoided.

    For instance of / is_a, Ive found this is usually an obvious case for a strategy pattern. Using these methods on an implementation creates a dependancy that usually isnt needed.

  23. #23
    SitePoint Guru
    Join Date
    May 2005
    Location
    Finland
    Posts
    608
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Brenden Vickery
    Private methods are just an indication to me that I *may* have a problem and should take a closer look. They are certainly not something to be avoided.
    Your example made sense. Thank you; I agree with your point.

    For instance of / is_a, Ive found this is usually an obvious case for a strategy pattern. Using these methods on an implementation creates a dependancy that usually isnt needed.
    I still don't quite follow. Both of those test for whether the class of the instance in question has a certain class in its inheritance tree; $c instanceof Controller returns true for both FrontController and PageController. Perhaps an example as to what you mean is again in place.

  24. #24
    SitePoint Addict
    Join Date
    May 2003
    Location
    Calgary, Alberta, Canada
    Posts
    275
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by beetle
    What? Why does this raise a flag for you? One of the basic powers of delegating processes to functions (et methods) is the ability to receive a return value. Be it a success state, value transformation, or something else altogether like a Factory.
    Take the City/Citizen example I gave above. Say the reason I need to get all private citizens named John is because marketing has told me that citizens like this are the most likely to buy my product and I should send them a email. So I write this code.
    PHP Code:
    class City {
        private 
    $citizens;
        private 
    $altitude;
        private 
    $name;
        public function 
    getPrivateCitizensNamedJohn() {
            
    $result = array();
            foreach(
    $this->citizens as $citizen) {
                if(
    $citizen->isPrivateCitizenNamedJohn()) {
                    
    $result[] = $citizen;
                }
            }
            return 
    $result;
        }
    }

    class 
    Citizen {
        private 
    $name;
        private 
    $email;
        private 
    $isPrivate;
        public function 
    isPrivateCitizenNamedJohn($citizen) {
            return (
    $this->isPrivate && $this->name == 'John');
        }
        public function 
    getEmail() {
            return 
    $this->email;
        }
    }

    $city $datasource->getCityNamed('London');
    $privateJohns $city->getPrivateCitizensNamedJohn();
    $productEmail = new ProductEmail();
    foreach(
    $privateJohns as $privateJohn) {
        
    $emailAddress $privateJohn->getEmail();
        
    $productEmail->sendTo($emailAddress);

    Code seems pretty good right? I think it could be better by removing as many functions that return anything and make them do something. Maybe something like this:
    PHP Code:
    class City {
        private 
    $citizens;
        private 
    $altitude;
        private 
    $name;
        public function 
    sendEmailToCitzensMatching($specification) {
            foreach(
    $this->citizens as $citizen) {
                
    $specification->sendEmailTo($citzen);
            }
        }
    }

    class 
    CitizensNameJohnEmailSpec {
        public function 
    sendEmailTo($citizen) {
            if(
    $citizen->isPrivateCitizenNamedJohn()) {
                
    $citizen->sendEmail($this->productEmail);
            }
        }
    }

    class 
    Citizen {
        private 
    $name;
        private 
    $email;
        private 
    $isPrivate;
        public function 
    isPrivateCitizenNamedJohn($citizen) {
            return (
    $this->isPrivate && $this->name == 'John');
        }
        public function 
    sendEmail($email) {
            
    $email->sendTo($this->email);
        }
    }

    $city $datasource->getCityNamed('London');
    $spec = new CitizensNameJohnEmailSpec(new ProductEmail());
    $city->sendEmailToCitizensMatching($spec); 
    This code does the same thing except we have alot more flexibility. This flexibility is gained by telling the method to do something instead of asking it for something. Pass in any spec to the sendEmailToCitizensMatching and it will send an email to those people matching that spec. You no longer need the getEmail method in Citizen you just pass the email you want to send. etc.

    Not all methods that return something are bad but like above they are something that should be looked at.

  25. #25
    Web-coding NINJA! silver trophy beetle's Avatar
    Join Date
    Jul 2002
    Location
    Dallas, TX
    Posts
    2,900
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Brenden, I think you make a good point but base it around the wrong argument: that methods that return "something" should be evaluated for "smell". That, in and of itself, is terribly misleading.

    What you're really talking about is changing those methods when composition can be applied to your class structure.

    So, to that point, I would agree with the promotion of composition where applicable.
    beetle a.k.a. Peter Bailey
    blogs: php | prophp | security | design | zen | software
    refs: dhtml | gecko | prototype | phpdocs | unicode | charsets
    tools: ide | ftp | regex | ffdev





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
  •