SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 46

Thread: OOP method

  1. #1
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    OOP method

    Some methods

    Im currently learning to better myself as a programmer who can create more effective and efficient OOP methods, that are both easy to use and versatile.

    Im currently writing a few methods to deal with commenting and posting etc.

    Having getComment, addComment, etc, effective and useful?

    I am talking over with a friend who is much more experienced and he talks more about how he does things in JAVA, etc. Im just wondering is there an easier way to layout these methods, with easy setter/getter methods.

    This is an attempt at another layout.

    Link

    Thank you in advanced.

  2. #2
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    As a first step I would take out the globals. Pass these items into the methods that need them, or if this is too much work then into the constructor as a last resort. Try not to hang on to data longer than necessary.

    Please make these changes and then post again. It is easier to do this one step at a time.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  3. #3
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hmmm OK. Well passing these objects through the functions is normal?

  4. #4
    SitePoint Enthusiast
    Join Date
    Jul 2003
    Location
    Switzerland
    Posts
    36
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    The first point I saw was the nested if-else structure :

    PHP Code:
     if ($auth->isIdentified) {
                    if (isset(
    $id)) {
                        ...
                        if (
    $auth->session['member_id'] == $data['member_id']) {
                        ....
                            @
    header ("Location: ".$_SERVER['HTTP_REFERER']."");
                            exit();
                        } else {
                            @
    header ("Location: ".WWW_PATH."");
                            exit();
                        }
                    } else {
                        @
    header ("Location: ".WWW_PATH."");
                        exit();
                    }
                } else {
                    @
    header ("Location: ".WWW_PATH."");
                    exit();
                } 
    Replace it with something similiar to this structure (if you can't avoid it)..
    PHP Code:
    if(!= b) {
        
    quit(1);
    }

    dosomething;

    if(
    != d) {
        
    quit(2);
    }

    somethingelse;

    if(
    != f) {
        
    quit(3);
    }

    again

  5. #5
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by ausurt
    Hmmm OK. Well passing these objects through the functions is normal?
    Mostly. When you find that this gets tedious you then take them in the constructor and access them as object wide variables. If this gets tedious then we resort to a registry pattern. If passing a registry around is getting tedious then make the registry a singleton.

    The point is that you reduce the scope as much as possible. That's the point of encapsulation. If you find that your $smarty object was corrupted, which class in your app. would you blame? With the object passed in you can test your classes with controlled input. In short, start small. You will never need a global.

    Anyway, this is just a first step and should result in the parts of your class becoming more seperated. Once we hack away unneeded entanglements, we will be ready to break it down into smaller pieces. What happens after this is quite astonishing if you are not used to OO.

    Anyway, please post a modified version and I'll go another step.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  6. #6
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

  7. #7
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    Not exactly as you are just taking in the globals secretly in the constructor .

    More like this...
    PHP Code:
    class Comments {
        function 
    Comments() { ... }
        function 
    getComments(&$mysql, &$auth, &$smarty$id$offset) { ... }
        function 
    addComment(&$mysql, &$auth$contents) { ... }
        function 
    editForm(&$mysql, &$auth, &$smarty$id) { ... }
        function 
    editComment(&$mysql$contents) { ... }
        function 
    delComment(&$mysql, &$auth$id) { ... }

    Currently your class is a well topped cheesy pizza. If you try to separate pieces of it you will find the pieces remain joined with strands of melted cheese and some of your vegetables drop off. What we want to do is gather like vegetables together so that we can cut it cleanly without stringy bits.

    OK, you have probably never had a class compared to a cheesy pizza before . I guess it's not a great analogy, but we cannot do a divide and conquer approach at the moment and that is a crippling problem. Once you have refactored out any input that you don't have control of (so no $_POST, $_GET or $_GLOBALS arrays) we can proceed to seperate off the viewing stuff.

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  8. #8
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hehe i found it interesting. Thought it wouldnt last, but it did, nice one.

    http://ausurt.silvs.net/tmp/comments.class.phps

    OK done that. Is there a better way to get in contact with you?
    Last edited by ausurt; Sep 2, 2003 at 23:00.

  9. #9
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    I was hoping for a more radical movement .

    Ok, rule 1 - No globals.

    You need to take out the global declarations in the methods as well and remove acces to the $_GET and $_POST arrays. E.g.
    PHP Code:
    function editComment (&$mysql, &$auth) {
        global 
    $mysql$auth;
        if (
    $auth->isIdentified) {
            if (empty(
    $_POST['comment']) OR !$_POST['editcom']) {
                @
    header ("Location: ".WWW_PATH."");
                exit();
            }
            ...
        }

    ...becomes...
    PHP Code:
    function editComment(&$mysql, &$auth$post) {
        if (!
    $auth->isIdentified()) {
            if (empty(
    $post['comment']) OR !$post['editcom']) {
                @
    header ("Location: ".WWW_PATH."");
                exit();
            }
            ...
        }

    Note that $auth->isIdentified() is a method. All it does is return the original variable, but there are good reasons for doing this. Later it may no longer be a simple flag, but something more complicated. That would mean that every script that uses the auth class would have to be changed. With the modified version all of these changes occour behind the shield of the method call. Client scripts are blissfully unaware of the change and so you incur no extra work. If this is part of a library then this alone is a good reason to junk the library and choose a different one. Encapsulation is the great enabler, so...

    Rule 2 - Don't access the innards of a class

    Because this could take along time otherwise I would like you to do something else as well. Whenever you see a chunk of code that looks like it works together I want you to extract it out into a seperate method.

    In the above fragment I would do this...
    PHP Code:
    function editComment(&$mysql, &$auth$post) {
        if (!
    $auth->isIdentified()) {
            if (!
    $this->_isEditRequest($post)) {
                
    $this->_redirectToHomepage();
            }
            ...
        }
    }

    function 
    _isEditRequest($post) {
        return (
    $post['comment'] && $post['editcom']);
    }

    function 
    _redirectToHomepage() {
        @
    header ("Location: ".WWW_PATH."");
        exit();

    Notice how this reads more naturally. It is much clearer to someone scanning the code. The methods are marked with underscores as a hint to the reader that they should not be run from outside that class. It's for internal use only.

    In summary...

    Rule 3 - No method longer than 10 lines.

    In fact five or six lines is more than enough. You now have some work to do and I'll be somewhat insulted if you brush me off with a few minor edits. I would like you to go through the code so that there is not a single breakage of any of these rules. Once you have done that we can stop worrying about encapsulation and move on to polymorphism, which is where the real power is.

    Quote Originally Posted by ausurt
    OK done that. Is there a better way to get in contact with you?
    Follow the link on the left to mail me directly, but I can visit sitepoint wherever I am, so carrying on as is is probably better. Also others can follow progress too and add their own comments.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  10. #10
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Yeh working through it now. Sorry about the globals bit under the function names, i thought i removed them all from there.

    OK done some editing. What would you think about a method for substituting smarty values?

    Code:
    Class mySmarty extends Smarty {
          function setValues ($input) {
              foreach ($input as $k => $v) {
                  $smarty->assign ($k, $v);
              }
          }
    }
    Rule #3.

    How would i go about getComments and shortening it?

    By the way thanks for the help so far.
    Last edited by ausurt; Sep 3, 2003 at 04:36.

  11. #11
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    Quote Originally Posted by ausurt
    OK done some editing. What would you think about a method for substituting smarty values?
    In general definitely a good idea and you are on the right track. You could remove the Smarty dependency all together by wrapping Smarty in another class rather than extending it, but that is another story.

    Quote Originally Posted by ausurt
    How would i go about getComments and shortening it?
    Looking for repetition is a good one. You have got lots of Smarty assignments of key-value pairs which you have already tackled. Another one is looking at common resources they use. Some parts use $auth, some $mysql and some $smarty. Splitting along these natural fault lines should be easier and you will have less parameters to pass. It doesn't have to be perfect, just manageable.

    yours, Marcus

    p.s. Top tip; instead of the CODE tag...
    Code:
    <?php print "Hello"; ?>
    ...try the PHP one for yukky colours...
    PHP Code:
    <?php print "Hello"?>
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  12. #12
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Been watching this from the start and I'd have to agree that you shouldn't use GLOBALs.

    Looking forwards to see OO Poly. since I've read about it but never used it so looking forward to some example script

  13. #13
    SitePoint Addict
    Join Date
    Aug 2002
    Location
    Ottawa, Ontario, Canada
    Posts
    214
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lastcraft
    Rule 3 - No method longer than 10 lines.
    This was a rather large sticking point recently on a couple lists I am on (a test driven development list and a refactoring list). I am not sure I agree with this as a "written in stone" rule, but I do agree with the sentiment the rule attempts to deliver

    Just my $0.02

    Cheers,
    Keith.

  14. #14
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    OK just woke up, done a bit. Just wanted to get some done before i get ready and go out.

    http://ausurt.silvs.net/tmp/comments.class.phps

  15. #15
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    HI.

    Quote Originally Posted by Taoism
    I am not sure I agree with this as a "written in stone" rule
    It isn't written in stone at all of course, but is useful here. I get suspicious of methods over 6 and start to feel uneasy at 8. I definitely lose track of what is happening at 10 hence that choice. Depends on language though. I can handle more Java or SQL, less with XSLT or C++. I guess it should really be...

    Rule 3 - Don't write a method larger than everyone on the team can completely understand.

    Doesn't have the same ring though .

    yours, Marcus
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  16. #16
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by ausurt
    OK just woke up, done a bit. Just wanted to get some done before i get ready and go out.
    I'll wait. Getting there though.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  17. #17
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hmmm can't think of howto handle some of the things.

    The extra $auth->isIdentified() in getComments to assign the form etc. :\

  18. #18
    SitePoint Guru
    Join Date
    Nov 2002
    Posts
    841
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by lastcraft
    Rule 3 - No method longer than 10 lines.
    Quote Originally Posted by Taoism
    This was a rather large sticking point recently on a couple lists I am on (a test driven development list and a refactoring list). I am not sure I agree with this as a "written in stone" rule, but I do agree with the sentiment the rule attempts to deliver
    Quote Originally Posted by lastcraft
    It isn't written in stone at all of course, but is useful here. I get suspicious of methods over 6 and start to feel uneasy at 8. I definitely lose track of what is happening at 10 hence that choice. Depends on language though. I can handle more Java or SQL, less with XSLT or C++. I guess it should really be...

    Rule 3 - Don't write a method larger than everyone on the team can completely understand.
    The measurement that you are looking for is called [McCabe's] cyclomatic complexity. It measures the number of branches through the code. Loosely, it is a measurement of the number of test cases that you would need to achieve branch coverage of a method.

    You can approximate it by counting all of the branching statements and adding one. Branching statements are: if, while, for, etc. There are two extra issues. Every logical && (and) and || (or) counts as one. Also, every case in a switch statement counts as one.

    For example the following code has a cyclomatic complexity of 3:

    PHP Code:
    if ($condition1 || $condition2) {
        
    $x 0;
        
    $y 0;
    } else {
        
    $x 1;
        
    $y 1;

    All your methods should have a cyclomatic complexity of less than 10. There is some academic research behind picking number 10 here. I will sometimes go as high as 12, but every time I have ever split up a method over 12, I have been pleased with the final results even if I didn't think it was necessary to split it up when I started.

    After practicing the cyclomatic complexity for a while, you will get to where you can just look at the code and see that it is too complex.

    One of the reasons for the success of test driven design is that it forces you to write methods that are testable. It turns out that testable methods are more understandable and less buggy independantly of the tests, just because of their smaller size. (you can't test long methods.)

    Cyclomatic Complexity lets you estimate the testability of a method without writing the tests. (although, writing the tests is better)

    Cyclomatic complexity is language independent, unlike lines of code.

    Why 10? Well, there has been some suggestion that it is related to the magic number 7 (+/- 2) from memory research. I think there is something to this idea, but miller's research is often abused out of context. 10 represents the upper limit. Most of your methods would be lower than 7 anyway.

  19. #19
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by Selkirk
    The measurement that you are looking for is called [McCabe's] cyclomatic complexity
    Nice one.

    Another factor could be inputs and outputs, for example...
    PHP Code:
    function doStuff(&$a$b, &$c$d) {
        
    $a $b $c;
        
    $d $b $c;
        return 
    $d $c +$b;

    ...would score one on branching, but has four inputs and 3 outputs. This would explain why XSLT is so damn tricky. It is highly context sensitive with inputs coming in all over the place (external variables, incoming variables, the XPath invocation and the mode).

    yours, Marcus.

    p.s. I see you have joined me in site point zealot status. Geek joy . I really must get out more .
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  20. #20
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    [Off Topic]

    Umm.... XSL-T it's self isn't tricky IMO ? A lot of folks do have trouble with it I admit although what complicates an XSL stylesheet even more is XPath...

    It's not easy so I always stay well back from using it

    Like you need a science degree just to get the basics

  21. #21
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I am just a member. :P

  22. #22
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi...

    Quote Originally Posted by ausurt
    Hmmm can't think of howto handle some of the things.

    The extra $auth->isIdentified() in getComments to assign the form etc. :\
    Then you have to pass $auth into that method. Don't worry, just get something up. Once the code is in small pieces it will be a lot easier to shuffle around. The class itself is going to get split next and then things will get easier. The main thing is to post a valid response quickly and get to the next step.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things

  23. #23
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Sorry, i did upload whatever i have changed. Just busy day. I havent looked at the code since that post.

    Something wrong with my net lately, keeps on going normal then really slow.

    Was looking at some stuff Dr Livingston posted in "OOP Problem: Data Mapper". Is this sort of the direction im heading towards?
    Last edited by ausurt; Sep 5, 2003 at 15:59.

  24. #24
    SitePoint Zealot
    Join Date
    Aug 2003
    Location
    Sydney
    Posts
    187
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Well i uploaded an updated version. All i've done really is remove the displaying stuff and just left the functions to return objects.

    Only look at it 10 minutes before this post, been a bit busy as of late.

  25. #25
    ********* Victim lastcraft's Avatar
    Join Date
    Apr 2003
    Location
    London
    Posts
    2,423
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    Can you post a link, or better yet paste in the code. That way the discussion is preserved for posterity.

    yours, Marcus.
    Marcus Baker
    Testing: SimpleTest, Cgreen, Fakemail
    Other: Phemto dependency injector
    Books: PHP in Action, 97 things


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
  •