Code Efficiency, Proper Use?
Results 1 to 6 of 6

Thread: Code Efficiency, Proper Use?

  1. #1
    Junior Member
    Join Date
    May 2007
    Posts
    15

    Code Efficiency, Proper Use?

    I'm wondering if perhaps I am doing this all wrong.. see for yourself:

    Question:

    • Does my OOP suck?
    • is running multiple methods on a value {ie: strip_tags($copy->truncateString($articles[$i]['body'], 250, " "))} a terrible way to manage resources?
    • Should I create seperate methods to use for the following line: {echo '' . $articles[$i]['title'] . ' (' . $articles[$i]['date'] . ')' .strip_tags($copy->truncateString($articles[$i]['body'], 250, " ")) . '

      Read more
      ';}?



    Notes:
    • pageTemplate is a class which will contain all html templats for this particular site
    • Two static classes take care of site wide configuration variables (not seen here) and tracking the url (BreadCrumbs:.
    • Calling BreadCrumbs::getCrumb(x) retrieves the 'value' at the x position of the url (ie: domain.com/"pos 1"/"pos 2"/"pos 3"). The values are managed at the top of the page and are checked if empty and replaced with a default value if they are.
    • GetCopy is a class which has a number of functions designed to run a query (based on various parameters) and retrieves an array (or mulch-dimensional array). The function used below retrieves a mulch-dimensional array with each second level array containing a whole article.
    • GetCopy has a parent. It extends a class called CopyMan which contains various functions such as adding paragraphs to a block of text, truncating, converting date forms, and pagination tools. Called through the GetCopy object.



    PHP Code:
        /**
         * homePage :: ()
         * PUBLIC method
         * = lay out the home page content
         *
         */

        
    public static function homePage() {

        
    // initialize GetCopy class
        
    $copy = new GetCopy();

        
    // special feature container
        
    echo '<div id="special_box"><div class="special_header"><img src="/images/layout/special_header.png"></div><div class="special_container">';
        
    $articles $copy->getRows('articles''date'02);
        
    //print_r($articles);
        
    for ($i 0$i count($articles); $i++) {
            echo 
    '<div class="special_summary"><a href="/' BreadCrumbs::getCrumb(1) . '/'BreadCrumbs::getCrumb(2) . '/article/' $articles[$i]['id'] . '"><h5>' $articles[$i]['title'] . '</h5></a> (' $articles[$i]['date'] . ')<p>' strip_tags($copy->truncateString($articles[$i]['body'], 250" ")) . '</p><p><a href="/' BreadCrumbs::getCrumb(1) . '/'BreadCrumbs::getCrumb(2) . '/article/' $articles[$i]['id'] . '"> Read more</a></p></div>';
        }
        echo 
    '</div></div>';
        } 
    Please keep in mind that I taught myself PHP on my own.. no mentors, or corporate structure, classes, or any theory.. just learned as I need to learn how to do things. So please offer constructive criticism, I'm sure I need it.

  2. #2
    Senior Member
    Join Date
    Apr 2003
    Location
    Silver Lake
    Posts
    4,887
    Quote Originally Posted by obmon View Post
    Does my OOP suck?
    Hard to say, you haven't posted much.

    Quote Originally Posted by obmon View Post
    is running multiple methods on a value {ie: strip_tags($copy->truncateString($articles[$i]['body'], 250, " "))} a terrible way to manage resources?
    I wouldn't say calling a function on your method is necessarily bad -- you might want to do it in your method if you find yourself doing it every single time -- or perhaps defining an optional parameter that does the strip_tags bit. Not sure how it relates to "managing resources." If you have to strip tags, that's a pretty decent way to do it and fairly efficient. If you don't have to strip tags, then yes, it's a waste.

    Quote Originally Posted by obmon View Post
    Should I create seperate methods to use for the following line: {echo '' . $articles[$i]['title'] . ' (' . $articles[$i]['date'] . ')' .strip_tags($copy->truncateString($articles[$i]['body'], 250, " ")) . '

    Read more
    ';}?
    Not sure what you're asking...should you define a method that does what all that code does? If you do it a lot, sure.

    I can't really comment on the notes you posted about your code, but I would suggest that it's not a pain in the ass to maintain echo statements with all kinds of HTML like you have. I find that whenever I end up doing this, I later wish that the HTML could have been separated from the logic that retrieves the data to be displayed. This is typically because HTML has evolved, smart phones have become popular (and they require a totally different layout) and also because HTML is not the only way I might want to display my content -- I might want RSS or XML or something else entirely.

    I hope this is helpful input. You obviously have a reasonably good command of classes and such. I don't know much about how your code works overall. You might want to check out CodeIgniter or Zend Framework -- they have some pretty clever code organization which you may find interesting.
    IMPORTANT: STOP using the mysql extension. Use mysqli or pdo instead.
    World War One happened 100 years ago. Visit Old Grey Horror for the agony and irony.

  3. #3
    Junior Member
    Join Date
    May 2007
    Posts
    15
    Quote Originally Posted by sneakyimp View Post
    I hope this is helpful input.
    Thanks for taking the time. I was going to post more of the code to show, but I was aiming for brevity. Below though, you can see the "structure" of the code lib that I am building (don't dare call it a framework yet, although that is the eventual goal).

    You're input is very helpful. Like you said, I should download another PHP Framework and just spend a few days analyzing their structure..

    Truth is, since I have had no one looking over my shoulder, ever, I really need someone to do that now. To look over my work and ensure I am on the right path. I guess best thing to do is offer payment to an Expert PHP developer to do just that. Thanks for your input though.


    CODE STRUCTURE: (http://codereview.stackexchange.com/...roper-code-use)
    -[Folder] Lib
    -[Folder] bespoke
    --> [Class] DisplayEngine
    private static $instance;
    public static function getInstance()
    + Methods for laying out divs.
    --> [Class] PageTemplate
    public function homePage()
    -[Folder] copy
    --> [Class] CopyMan
    public function truncateString($string, $limit, $break='.', $pad='...')
    public function convertToPara($string)
    public function convertToLongDate($date)
    protected function paginateList($table, $current_page, $list_length)
    protected function setLimits($table, $page, $numArticles, $featured)
    --> [Class] GetCopy extends CopyMan
    require_once(Config::getAbsPath() . '/lib/omc_frmwrk/copy/CopyMan.php');
    public function getFieldContents($table, $where_field, $where_match, $column)
    public function getRow($table, $field_array, $where_field, $where_match)
    public function getRows($table, $order_by, $limit_start, $limit_end)
    public function getRowsWhere($table, $order_by, $where_field, $where_match, $limit_start, $limit_end)
    -[Folder] database
    --> [Class] DbMan
    private static $db_host = Config::db_host;
    private static $db_user = Config::db_user;
    private static $db_pass = Config::db_pass;
    private static $db_name = Config::db_name;
    private $mysql_connection;
    private $mysql_db;
    private $query;
    private $query_result;
    private function dbConnect()
    protected function executeQuery($query)
    --> [Class] DbQuery extends DbMan
    private $query;
    public function prepareQuery($query_request)
    -[Folder] helpers
    --> [Class] ArrayHelp
    public function recValueCheck($needle, $haystack)
    public function recValueReturn($needle, $haystack)
    -[Folder] layout
    --> [Class] LogoMan
    private static $site_logo;
    public static function setSiteLogo($x) { self::$site_logo = $x; }
    public function printLogo()
    -[Folder] navigation
    --> [Class] BreadCrumbs
    private static $bread_crumbs;
    private static $instance;
    public static function getCrumbs() { return self::$bread_crumbs; }
    public static function getCrumb($x) { return self::$bread_crumbs[$x]; }
    public static function setCrumbs($x) { self::$bread_crumbs = explode("/", $x); }
    public static function getInstance() { if (!self::$instance) { self::$instance = new self(); } return self::$instance; }
    public static function setEmptyCrumb($crumb, $fallback)
    --> [Class] NavMan
    private static $links;
    private static $nav_id;
    public static function setLinks($x) { self::$links = $x; }
    public static function setNavId($x) { self::$nav_id = $x; }
    public function printNav($pre_link = "", $post_link = "")
    --> [Class] Config
    const db_host =
    const db_user =
    const db_pass =
    const db_name =
    private static $abs_path;
    private static $include_css = array ()
    private static $include_classes = array ()
    private static $include_js = array ()
    private static $site_logo = array ()
    private static $nav_primary = array ()
    private static $nav_secondary = array ()
    + Getters and Setters for all above
    --> [Class] initialize
    private static $include_css;
    private static $include_classes;
    private static $include_js;
    public function __construct() //*sets session, runs include methods below *//
    private function loadStylesheets()
    private function loadRequiredClasses()
    public function loadJavascript()

  4. #4
    Senior Member
    Join Date
    Apr 2003
    Location
    Silver Lake
    Posts
    4,887
    As much as I hate to suggest it, you might consider a book on coding? I've got this one and have only peeked in it a couple of times:
    http://www.amazon.com/Objects-Patter...ref=pd_sim_b_1
    A lot of such books you see are for beginners. This one not so much.
    IMPORTANT: STOP using the mysql extension. Use mysqli or pdo instead.
    World War One happened 100 years ago. Visit Old Grey Horror for the agony and irony.

  5. #5
    Junior Member
    Join Date
    May 2007
    Posts
    15
    Not sure if that book will help. I learned through purely practical try-fail methods, and according to the reviews this book is more theory than practice. If I find it though, i will look at it.

  6. #6
    Senior Member
    Join Date
    Apr 2003
    Location
    Silver Lake
    Posts
    4,887
    That you are seeking a code critique suggests that some theory might be helpful. If you are looking for some good code examples, you might consider a cookbook-type book -- or go check out a framework as we mentioned before. Books can really help you get your mind around why things are done a particular way (e.g., they might describe memory usages or common errors for a certain approach). Looking at well-designed code can really open your mind to better ways of doing things and help fill up your bag of code tricks.
    IMPORTANT: STOP using the mysql extension. Use mysqli or pdo instead.
    World War One happened 100 years ago. Visit Old Grey Horror for the agony and irony.

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
  •