Method Chaining = Poor OOP practice? - Page 2
Page 2 of 2 FirstFirst 12
Results 16 to 24 of 24

Thread: Method Chaining = Poor OOP practice?

  1. #16
    PHP Witch laserlight's Avatar
    Join Date
    Apr 2003
    Location
    Singapore
    Posts
    13,561
    Quote Originally Posted by Bonesnap
    Also I think number 4 doesn't make much sense. Either you have lots of code in few files, or few files with lots of code. It really depends on the project, and I think it's too general a statement.
    I think the statement is badly phrased, but the general idea is that if you have files that contain way too much code, it probably means that the code is not as modular as it should be to make it easier to understand and maintain. In this sense, "thousands of lines" is not an absolute metric, but a possible guideline. As with all design, there is the element of it being subjective and involving some trade-off.

    Quote Originally Posted by traq
    I think the implication is that the program flow is difficult to follow, and so requires a running commentary in order to make any sense of it. I agree in that case; though I certainly don't agree that code comments are a bad thing in and of themselves.
    Indeed. It probably means that the code should have been broken up into smaller, well named functions. Within those smaller functions, it may be necessary to have a comment to explain something that is not evident in the code, but that is a good, not bad, use of comments.
    Use Bazaar for your version control system
    Read the PHP Spellbook
    Learn How To Ask Questions The Smart Way

  2. #17
    Senior Member
    Join Date
    Mar 2009
    Posts
    802
    Quote Originally Posted by traq View Post
    I think the implication is that the program flow is difficult to follow, and so requires a running commentary in order to make any sense of it. I agree in that case; though I certainly don't agree that code comments are a bad thing in and of themselves.
    Makes sense. To be honest I am probably too liberal with inline comments; I just find it much easier to go back to a project six months old and fix a bug or make changes and instead of translating the code, I can easily read through some of the inline comments and pick up where I left off without any hassle. I know some would argue that the code should be simple enough, but the truth is I go through so many projects that they begin to bleed into one another, and the comments do a much better job at jogging my memory. One thing I would love to see in an IDE, is the ability to toggle the display of comments (both inline and otherwise). For the longest time (many moons ago) comments were very distracting to me, so naturally I didn't write any, or would "do it later" which either didn't happen, or they were useless because by the time I went back to write them, I was so detached from the code the quality just wasn't there.

    Quote Originally Posted by laserlight View Post
    I think the statement is badly phrased, but the general idea is that if you have files that contain way too much code, it probably means that the code is not as modular as it should be to make it easier to understand and maintain. In this sense, "thousands of lines" is not an absolute metric, but a possible guideline. As with all design, there is the element of it being subjective and involving some trade-off.
    Yeah I definitely try to take the modular approach. I like breaking code into different files that serve different purposes. Some coworkers of mine like to write their WordPress plugins in a single file (much easier to copy and paste, amirite?), but I can't stand this; I have to break it up into at least three files (they all have their purpose). On the other hand though, nothing's more annoying than having to open like 12 files to trace where some variable or function is being called. As you said, some trade-off is required.
    Declare variables, not war.

  3. #18
    Senior Member traq's Avatar
    Join Date
    Jun 2011
    Location
    so.Cal
    Posts
    949
    Quote Originally Posted by Bonesnap View Post
    To be honest I am probably too liberal with inline comments; I just find it much easier to go back to a project six months old and fix a bug or make changes and instead of translating the code, I can easily read through some of the inline comments and pick up where I left off without any hassle.
    That's different - you're commenting on something that actually needs a comment:
    PHP Code:
    // I know this looks correct, but it breaks on Monday mornings if there was a full moon the night before
    // $answer = 1+2 / 4;  
    $answer is_Sunday_with_Full_Moon()? 1+abs( -): 1+4;  // reminder - DO NOT change it back! 
    PHP Code:
    $status 1;  // this works for now but needs to be fixed later 
    Both valid uses of comments.

  4. #19
    Senior Member
    Join Date
    Mar 2009
    Posts
    802
    Quote Originally Posted by traq View Post
    That's different - you're commenting on something that actually needs a comment:
    Haha I appreciate the vote of confidence, but I am not sure all the time. Here's a function I wrote to assist in a WordPress project that's going live on Monday. I think it demonstrates my typical use of inline comments. You be the judge.

    PHP Code:
    /**
     * Gets posts based on a variable number of post meta key/value pairs. This function also takes an
     * optionsal associative array of post parameters like the standard get_posts() function; however,
     * only some of the fields are supported. The following fields are supported (with their defaults in
     * brackets):
     *
     * order_by (post_date)
     * order (DESC)
     * post_type (post)
     * post_status (publish)
     *
     * Meta keys will be member variables on returned objects. Their values will be the corresponding
     * meta values.
     *
     * @version 1.0
     * @param array $metas An associative array of meta key/value pairs to match.
     * @param array $params The standard associative array of parameters that are used for get_posts().
     * @param int $offset The offset from the first row.
     * @param int $max The maximum number of cars to return. Last 2 arguments are optional but are required when used together.
     * @return array|false Returns an array of standard class objects on success or false otherwise.
     */
    function get_posts_multiple_metas($metas$params=array(), $offset=0$max=0)
    {
        
    //Make sure we're working with arrays with minimum requirements
        
    if(!is_array($params) || !is_array($metas))
        {
            return 
    false;
        }
        
        global 
    $wpdb;
        
    $posts     = array();
        
    $query     '';
        
    $offset    = (int)$offset;
        
    $max       = (int)$max;
        
        if(
    $offset || $max 0)
        {
            
    $limit " LIMIT ".$offset.", ".$max;
        }
        else
        {
            
    $limit '';
        }
        
        
    //Check if certain values should use their defaults or user-specified values
        
    $order_by     = (isset($params['order_by']))     ? $params['order_by']         : 'post_date';
        
    $order        = (isset($params['order']))         ? $params['order']         : 'DESC';
        
    $post_type    = (isset($params['post_type']))     ? $params['post_type']         : 'post';
        
    $post_status    = (isset($params['post_status']))     ? $params['post_status']     : 'publish';
        
        
    //Begin creating the custom query
        
    $query "SELECT posts.*,";
        
        
    //Select the metas
        
    foreach($metas as $key => $value)
        {
            
    $query .= $key.".meta_value AS `".$key."`,";
        }
        
        
    //You need to remove the rightmost comma otherwise you will have a SQL syntax error
        
    $query rtrim($query',')." FROM $wpdb->posts AS posts,";
            
        
    //Setup the postmeta
        
    foreach($metas as $key => $value)
        {
            
    $query .= "$wpdb->postmeta AS `".$key."`,";
        }
        
        
    //You need to remove the rightmost comma otherwise you will have a SQL syntax error
        
    $query rtrim($query',')." WHERE posts.post_type = %s AND posts.post_status = %s";
        
        
    //Connect posts table to postmeta table
        
    foreach($metas as $key => $value)
        {
            
    $query .= " AND ".$key.".post_id = posts.ID";
        }
        
        
    //Setup meta keys
        
    foreach($metas as $key => $value)
        {
            
    $query .= " AND ".$key.".meta_key = '".$key."'";
        }
        
        
    //Setup meta values
        
    foreach($metas as $key => $value)
        {
            
    $query .= " AND ".$key.".meta_value = '".$value."'";
        }
        
        
    $query .= $limit;
        
        
    $posts $wpdb->get_results($wpdb->prepare($query$post_type$post_status));
        
        return (
    $posts) ? $posts false;

    Looks like my tabbing isn't showing up perfectly here. Oh well, you get the idea.
    Declare variables, not war.

  5. #20
    Senior Member
    Join Date
    Sep 2011
    Posts
    258
    Thanks for the discussion guys, I think I've come out with a conclusion of Method Chaining as an OOP practice. Id say Method Chaining can be either an anti-pattern or not depending on the context, or more precisely, whether the class designed for using method chaining introduces dependency between methods. One perfectly legal case for method chaining is the usage of setter methods, as setter methods usually do not depend on one another:

    PHP Code:
    $object->setProperty1()->setProperty2()->setProperty3(); 
    In the setter method example, you can flip the order of setProperty1(), setProperty2() and setProperty3(), the script will still work. In fact, it is perfectly fine even if you remove setProperty1() from the script and it will still be able to set the other two properties nicely. Perhaps this is what you guys mean by fluent interface of setter methods? They look quite nice to me too, unless one setter method depends on another.

    I see one frequent usage of method chaining is to build a query object, which can be tricky. As your query object may use lots of chaining methods like select(), where(), limit(), order(), you may have two ways to accomplish this task. The first approach is to store a query string in the query object, and dynamically concatenates this string whenever a method is called on the query object. The second approach, on the other hand, stores different parts of the query string in separate query properties, while in the end these are combined to form a complete query string when the client coder uses the fetch() method to return records. The first approach is a poor OOP practice as it introduces dependency between each query methods, if you swap the position of select(), where(), and limit(), the resulting query string can be invalid and triggering an error. The second approach, however, essentially does not violate the law of demeter as you can call these query methods in any order, while still generating the correct results:

    PHP Code:
    $query->select($table)->where($whereClause)->limit($limit)->order($order); // returns valid records for both approaches
    $query->limit($limit)->order($order)->where($whereClause)->select($table); // may only return valid records for second approach 
    I'd say a rule of thumb is to swap the order of these chained methods being called, and see whether the script is still capable of generating appropriate results. If so, your method chaining does not violate law of demeter as you are simply providing a fluent interface rather than introducing dependency between methods. Otherwise, its a good idea to get rid of these chained methods as unwanted dependencies breaks the law of demeter rule.
    Last edited by Lord Yggdrasill; 09-07-2013 at 10:35 AM.

  6. #21
    Senior Member traq's Avatar
    Join Date
    Jun 2011
    Location
    so.Cal
    Posts
    949
    Quote Originally Posted by Lord Yggdrasill View Post
    I'd say a rule of thumb is to swap the order of these chained methods being called, and see whether the script is still capable of generating appropriate results. If so, your method chaining does not violate law of demeter as you are simply providing a fluent interface rather than introducing dependency between methods. Otherwise, its a good idea to get rid of these chained methods as unwanted dependencies breaks the law of demeter rule.
    Sounds like a good test to me.

  7. #22
    Pedantic Curmudgeon Weedpacket's Avatar
    Join Date
    Aug 2002
    Location
    General Systems Vehicle "Thrilled To Be Here"
    Posts
    21,863
    Quote Originally Posted by traq View Post
    Quote Originally Posted by Lord Yggdrasill
    I'd say a rule of thumb is to swap the order of these chained methods being called, and see whether the script is still capable of generating appropriate results. If so, your method chaining does not violate law of demeter as you are simply providing a fluent interface rather than introducing dependency between methods. Otherwise, its a good idea to get rid of these chained methods as unwanted dependencies breaks the law of demeter rule.
    Sounds like a good test to me.
    Of course, being a rule of thumb means it's easy enough to find counterexamples:
    PHP Code:
    $twoD_object->rotate(30)->rotate(-20); 
    would be acceptable, but
    PHP Code:
    $threeD_object->rotateX(30)->rotateY(-20); 
    would not.
    THERE IS AS YET INSUFFICIENT DATA FOR A MEANINGFUL ANSWER
    FAQs! FAQs! FAQs! Most forums have them!
    Search - Debugging 101 - Collected Solutions - General Guidelines - Getting help at all

  8. #23
    Senior Member traq's Avatar
    Join Date
    Jun 2011
    Location
    so.Cal
    Posts
    949
    I think that follows the example, doesn't it? Since swapping the rotateX and rotateY method calls leads to a different result, it would mean that our "rule of thumb" caught an inappropriate dependency introduction ...right?

    (In general, though, I agree. "Rules of thumb" are, by definition, not absolute.)

  9. #24
    High Energy Magic Dept. NogDog's Avatar
    Join Date
    Aug 2006
    Location
    Ankh-Morpork
    Posts
    13,915
    I find this statement on the Wikipedia page about the LoD kind of interesting, suggesting it's at best something that might flag a problem, versus an actual "law" that must be followed:
    Since the LoD exemplifies a specific type of coupling, and does not specify a method of addressing this type of coupling, it is more suited as a metric for code smell as opposed to a methodology for building loosely coupled systems.
    Please give us a simple answer, so that we don't have to think, because if we think, we might find answers that don't fit the way we want the world to be." ~ from Nation, by Terry Pratchett

    "But the main reason that any programmer learning any new language thinks the new language is SO much better than the old one is because hes a better programmer now!" ~ http://www.oreillynet.com/ruby/blog/...ck_to_p_1.html


    eBookworm.us

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Posting Permissions

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