Time to move to OOP?

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Time to move to OOP?

Post by psurrena »

Below is a general example of a typical page I would create. Does anyone see it benefiting from an OO approach? This page is called into a template system already. I just want a reason to start learning OOP and need a real example to do so. Any help would be greatly appreciated. (note, there are some consistency issues I'm aware of. I'm rewriting the page)

Code: Select all

 
<?php
    ## WORK item
    
    /*************************************************************/
    $pid=$_GET['id'];
    require_once(DB);
    
    $query="SELECT P.project_id, P.project_name, P.project_des, P.location_id, C.category_full, C.category_id 
    FROM project P, category C, project_category B 
    WHERE P.project_id='$pid' AND C.category_id=B.category_id";
    
    $result=mysql_query($query) OR DIE (mysql_error());
    list($pid,$pname,$pdes,$plid,$cfull,$cid)=mysql_fetch_array($result);
    
    //Display name and description
    if (mysql_num_rows($result)){
        echo "<h3>{$pname}</h3>\n";
        echo "<p>{$pdes}</p>\n";
        
        //Display project images
        $path="images/work/{$pid}/";
        $dir=opendir($path);
        while($name=readdir($dir)){
            if (preg_match("/t+([0-9]+\.jpg)$/",$name,$sp)){
                $nameb=ereg_replace('t','', $name);
                echo '<a href="'.BASE_URL.$path.$sp[1].'" rel="lightbox" title="'.$pname.'">';
                echo '<img class="work" src="'.BASE_URL.$path.$name.'" alt="'.$pname.'" width="130" />';
                echo "</a>&nbsp;\r";
            }
        }   
 
    /*************************************************************/
    //DEATAILS Section
        
        echo "<p><strong>Details</strong><br />\n";
        
        //Categories the project is listed under
        echo "Category: ";
        $query="SELECT C.category_full, C.category_id FROM category C, project_category P 
        WHERE C.category_id=P.category_id 
        AND 
        P.project_id='$pid'";
        
        $result=mysql_query($query) OR DIE (mysql_error());
        if(!mysql_num_rows($result)){
            echo "";
        }else{
            while(list($cfull,$cid)=mysql_fetch_array($result)){
                echo '<a class="workU" href="'.BASE_URL."category-{$cid}.html\">{$cfull}</a>\n";
            }
        }
        
        echo "<br />\n";
        
        // Locaton of the project
        echo "Location: ";
        $query="SELECT C.city_name, C.city_id, S.state_abbr, S.state_id 
        FROM project P, location_city C, location_state S 
        WHERE P.location_id=C.city_id 
        AND 
        C.state_id=S.state_id 
        AND 
        P.project_id='$pid'";
        
        $result=mysql_query($query) OR DIE (mysql_error());
        if(!mysql_num_rows($result)){
            echo "";
        }else{
            list($city,$cid,$state,$sid)=mysql_fetch_array($result);
            $cityd=ereg_replace(' ','-', trim($city));  
            echo '<a class="workU" href="'.BASE_URL."city-{$cityd}-{$cid}.html\">{$city}</a>, ";
            echo '<a class="workU" href="'.BASE_URL."state-{$state}-{$sid}.html\">{$state}</a></p>\n";
        }
        
        //PEOPLE
        $query="SELECT GROUP_CONCAT(membership_name ORDER BY M.membership_id SEPARATOR ', ') AS membership_name, P.people_id, people_fname, people_mname, people_lname, PR.project_id
        FROM membership M,people_membership PM,people P, project PR, people_project PP
        WHERE M.membership_id=PM.membership_id
        AND
        P.people_id=PM.people_id
        AND
        P.people_id=PP.people_id
        AND
        PP.project_id=$pid
        GROUP BY
        P.people_id
        ORDER BY P.people_lname ASC";
            
        $result=mysql_query($query) or die (mysql_error());
        if(!mysql_num_rows($result)){
            echo "";
        }else{
            echo "<p><strong>People</strong><br />";
            while(list($membership,$pid,$fname,$mname,$lname,$prid)=mysql_fetch_array($result)){
                echo '<a class="workU" href="'.BASE_URL."bio-{$fname}-{$lname}-{$pid}.html\">{$fname} ";
                if($mname) echo "{$mname}. ";
                echo "{$lname}</a><br />\n";
            }
            echo "</p>\n";
        }
    }else{
        echo 'There are no additional details available.';
    }
?>
 
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Re: Time to move to OOP?

Post by superdezign »

psurrena wrote:I just want a reason to start learning OOP and need a real example to do so.
I didn't feel like reading the code, but if you want to start learning OOP, start with creating objects for more concrete portions of your website. You don't need to start building a framework from the start, just start with more obvious concepts and work your way down to the more complex and abstract concepts.
User avatar
kaszu
Forum Regular
Posts: 749
Joined: Wed Jul 19, 2006 7:29 am

Post by kaszu »

1. Grouping your code into functions & classes would help readability, debuging, maintanance! Your code will be more structured.
2. These functions and classes then could be reused in other parts of the project or other projects.
3. Filtering and data validation could be done in classes, so you wouldn't have to worry about it when passing data from using input <- your script is vulnerable to SQL injections at the moment!!!
4. Other reasons, what currently i can't think of...

Very simple example:

Code: Select all

class Project {
    function setId($id) { ... }  //set id for project which will be loaded. Also you can do here validation, filtering or whatever.
    function setSomeOtherProjectValue($value) { ... }  //the same but for other variable
    function loadFromDb() { ... }  //loads info from database and returns an array
}
 
//Then you can use:
 
$project = new Project();
$project->setId($_GET['id']);
$result = $project->loadFromDb();
 
HTML currently is in your php code, i suggest to move all of it into separate file.
User avatar
jimthunderbird
Forum Contributor
Posts: 147
Joined: Tue Jul 04, 2006 3:59 am
Location: San Francisco, CA

Re:

Post by jimthunderbird »

HTML currently is in your php code, i suggest to move all of it into separate file.
Yes, I couldn't agree more! Try separating your application logic from the presentation logic, this is a good practice.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Time to move to OOP?

Post by Christopher »

psurrena wrote:... Does anyone see it benefiting from an OO approach? ... I just want a reason to start learning OOP and need a real example to do so.
The only real reason to change is because you need to. If you find this style of code solves the problems you have and is maintainable -- then why change? Others seeing any benefits would be benefits for them, not you. What you show is a Transaction Script. That is a fine pattern to use as long as it works for you, and you find the trade-offs acceptable.

When Transaction Scripts are no longer working for you, come back with the problems you are having and ask again.
(#10850)
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Re: Time to move to OOP?

Post by Kieran Huggins »

OOP may or may not be faster to write than spaghetti code, but it's a hell of a lot easier to maintain! When you work with a codebase for long enough to have to change, fix, migrate, etc... you'll get a serious return from OOP.

If you're doing something quickly, once, by yourself, you'll see little difference.

That being said, OOP is definitely worth having in your skill set.
User avatar
BDKR
DevNet Resident
Posts: 1207
Joined: Sat Jun 08, 2002 1:24 pm
Location: Florida
Contact:

Re: Time to move to OOP?

Post by BDKR »

I'm really big on the design side. Not too much I hope, but I feel that it's more important then the paradigm.

Do a little reading on Seperation of Concerns and/or Responsibility Driven Design. Try to imagine what you come away with being applied whether or not you are using objects proper. I promise it will make a better programmer out of you.

So to answer your question, I'd say that if you are starting to think about, it's time to move to Object Oriented Design (SofC/RDD), THEN OOP. Or perhaps both at the same time? Don't forget to do some digging into algorithms and data structures. :D
Post Reply