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.

    /**
     * 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', 0, 2);
//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.

    12 days later
    obmon;11021053 wrote:

    Does my OOP suck?

    Hard to say, you haven't posted much.

    obmon;11021053 wrote:

    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.

    obmon;11021053 wrote:

    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.

      sneakyimp;11021755 wrote:

      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/questions/19888/php-learning-oop-proper-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()

        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.

          8 days later

          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.

            Write a Reply...