TalkPHP
 
 
Account Login
Latest Articles
» The basic usage of PHPTAL, a XML/XHTML template library for PHP
» Vulnerable methods and the areas they are commonly trusted in.
» Simple way to protect a form from bot
» The Basics On: How Session Stealing Works
» How to keep your forms from double posting data
IRC Channel
IRC Speech Bubble Join the friendly bunch on IRC...
(#TalkPHP on Freenode)

...Also available via a web interface.

See this thread for information on the TalkPHP Free Hugs Initiative™. Subject to availability.
Associates
Associates
CSS Tutorials
Reply
 
LinkBack Thread Tools Search this Thread Display Modes
Old 05-08-2009, 10:20 PM   #1 (permalink)
The Visitor
 
Join Date: May 2009
Posts: 2
Thanks: 0
frostyboy33 is on a distinguished road
Default feedback on my class please

Hi guys

this is actually my first ever post on any type of php form so i hope i am doing it right. also i am only fairly new to the php oop style of thing and would appreciate any type of feed back on what i have done thus far. anything that u think could be done to improve my class or make it more "powerful"

quick run down this class is designed to generate a very basic table

also i haven't 100% tested every possible bug just made some changes this morning before i posted

php Code:
class NewCreateTable
{
    //! will hold the current table name of the table being built
    private $current_table;
    //! will hold the number of headings
    private $heading_count;
    //! array holding all errors encountered
    private $errors = array();

    // static variables
    //! Needed in order to make this class a singleton
    private static $CreateTable;

    //! Creates a single instance of the class
    /*!
     * \return The Static member holding this class
     */

    public static function getInstance()
    {
        if(!self::$CreateTable)
        {
            self::$CreateTable = new NewCreateTable;
        }

        return self::$CreateTable;
    }

    //! Ctor only accesible through getInstance()
    /*!
     */

    private function NewCreateTable()
    {

    }

    //! main table function will manage the creation of the table
    /*!
     * \param $table_name: The name of the table
     * \param $table_headings: The table headings single array
     * \param $table_data: The table data either an array or array full of arrays
     * \return the entire table in a string
     */

    public function cBasicTable($table_name, $table_headings, $table_data)
    {
        //**************************
        $table_string = NULL;       //string for the overall table
        //**************************

        // reset particular member properties
        $this->rProperties();

        // start the table
        $this->bStart($table_string,$table_name);

        // build the headings
        $this->bHeadings($table_string, $table_headings);

        // build the data
        $this->bData($table_string, $table_data);

        // end the table
        $this->bEnd($table_string);
       
        // check for errors
        $this->checkError($table_string);

        // return whatever has been created
        return $table_string;
    }

    //! will begin the table
    /*!
     * \param $table_string: The string to create the table
     * \param $table_name: The name of the table
     */

    private function bStart(&$table_string, &$table_name)
    {
        // set the table name
        $this->current_table = $table_name;
        // start the table
        $table_string = "<table name='". $table_name ."'>";       
    }

    //! will end the table
    /*!
     * \param $table_string: The string to create the table
     */

    private function bEnd(&$table_string)
    {
        // end the table
        $table_string = $table_string."</table>";
    }


    //! will build the headings for the table
    /*!
     * \param $table_string: The string to create the table
     * \param $table_headings: Array holding the table headings
     */

    private function bHeadings(&$table_string, &$table_headings)
    {
        //**************************
        $key = NULL;                // looping key
        $val = NULL;                // looping value
        //**************************
       
        if(is_array($table_headings))
        {
            // start the row
            $table_string = $table_string."<tr>";
            // loop through the array continaing the headings
            foreach($table_headings as $key => $val)
            {
                // create the heading
                $table_string = $table_string."<th>".$val."</th>";
                // add one to the headings count
                ++$this->heading_count;
            }
            // end the row
            $table_string = $table_string."</tr>";
        }
        else
        {
            // give a meaningful error message
            $this->errors[] = "ERROR(table ".$this->current_table."): table data is not in a readable format bHeadings()";
        }
    }

    //! will decide how to build the data for the table
    /*!
     * \param $table_string: The string to create the table
     * \param $table_data: either a single array or an array full of arrays containing the data for the table
     */

    private function bData(&$table_string, &$table_data)
    {
        //**************************
        $test_for_array = NULL;     // testing varriable
        //**************************

        // check to see if $table_data it self is an array
        if(is_array($table_data))
        {
            // create a test variable to see if $table_data is a single array or an array full of arrays
            $test_for_array = $table_data[0];
            if(is_array($test_for_array))
            {
                // if it's an array full of arrays
                $this->bDataArray($table_string, $table_data);
            }
            else
            {
                // if it's a single array
                $this->bDataNotArray($table_string, $table_data);
            }
        }
        else
        {
            // give a meaningful error message
            $this->errors[] = "ERROR(table ".$this->current_table."): table data is not in a readable format bData()";
        }
    }

    //! will build the data for the table with an array holding arrays
    /*! Each sub array will represent a new row
     * \param $table_string: The string to create the table
     * \param $table_data: an array containing arrays with the data
     */

    private function bDataArray(&$table_string, &$table_data)
    {
        //**************************
        $key = NULL;                // looping key
        $row = NULL;                // looping value(is an array)
        $key2 = NULL;               // looping key
        $val2 = NULL;               // looping value
        //**************************

        // cycle through the holding array
        foreach($table_data as $key => $row)
        {
            // will check every row to make sure it will line up with headings
            if(count($row) == $this->heading_count)
            {
                // begin the row for the current record
                $table_string = $table_string."<tr>";
                // cycle through each row for there data
                foreach($row as $key2 => $val2)
                {
                    // create the string for the data
                    $table_string = $table_string."<td>".$val2."</td>";
                }
                // end the row for the current record
                $table_string = $table_string."</tr>";
            }
            else
            {
                // give a meaningful error message
                $this->errors[] = "ERROR(table ".$this->current_table."): Data count does not match heading count in function bDataArray()";
                // exit the current cycle
                break;
            }
        }
    }

    //! will build the data for the table with a single array
    /*! will create a new row after a specified amount
     * \param $table_string: The string to create the table
     * \param $table_data: an array containing the data
     */

    private function bDataNotArray(&$table_string, &$table_data)
    {
        //**************************
        $data_count = NULL;         // holds the count of the $table_data array
        $division = NULL;           // holds the result of a division
        $row = NULL;                // looping counter(overall)
        $data = NULL;               // looping counter
        //**************************


        // see how manyrecords there are all together
        $data_count = count($table_data);
        // divide all records by the number of headings
        $division = $data_count / $this->heading_count;

        // $division should be an int(whole number no decimal) to match up with number of headings
        if(is_int($division))
        {
            // varriable to hold the overall record the loop will be on
            $row = 0;
            // $row need to equal $datacount to ensure all records ahve been displayed
            while($row < $data_count)
            {
                // begin the row for the current record
                $table_string = $table_string."<tr>";
                // internal loop to loop thorugh the row's records
                for($data = 0; $data < $this->heading_count; $data++)
                {
                    // create the string for the data
                    $table_string = $table_string."<td>".$table_data[$row]."</td>";
                    // increase overall record count by one
                    $row++;
                }
                // end the row for the current record
                $table_string = $table_string."</tr>";
            }
        }
        else
        {
            // give a meaningful error message
            $this->errors[] = "ERROR(table ".$this->current_table."): Data count does not match heading count in function bDataNotArray()";
        }
    }

    //! will check to see if there are any errors
    /*! if so the table string will be destroyed
     * \param $table_string: The string to create the table
     */

    private function checkError(&$table_string)
    {
        // check to see if we have any errors
        if($this->error != NULL)
        {
            // if there are unset the current table string
            unset($table_string);
            // set the table string to be the error array
            $table_string = $this->error;
        }
    }

    //! required everytime a new table is to be created
    /*!
     *
     */

    private function rProperties()
    {
        // make sure there is no old table name
        $this->current_table = NULL;
        // set headings count to 0 as every table could have a diffrent amount of headings
        $this->heading_count = 0;
        // make sure there are no left over errors from previous tables
        $this->errors = NULL;
    }
}

Last edited by Wildhoney : 05-09-2009 at 03:18 AM.
frostyboy33 is offline  
Reply With Quote
Old 05-09-2009, 09:58 AM   #2 (permalink)
The Prestige
Upcoming Programmer Inquisitive 
 
Tanax's Avatar
 
Join Date: Sep 2007
Location: Sweden, Stockholm
Posts: 1,080
Thanks: 115
Tanax is on a distinguished road
Default

One thing I notice was that since you're coding this for PHP5(using private scope for example), you should also use __construct instead of NewCreateTable
__________________
Tanax is offline  
Reply With Quote
Old 05-10-2009, 07:39 PM   #3 (permalink)
Jay
The Contributor
Good Samaritan 
 
Join Date: Dec 2007
Posts: 60
Thanks: 5
Jay is on a distinguished road
Default

Howdy ;)

1. Use brackets after the object name when creating new stuff: new object();

2. I see a lot of incorrect string concatenation -- well it's not correct, but it's also not efficient either: use the operand .=, like such:

PHP Code:
// BAD: $table_string = $table_string."</table>";
$table_string .= '</table>'
3. You should consider using single quotes for the simple fact that there's a slight performance increase when doing so. When using double quotes PHP checks for variables contained inside those quotes. The performance gain is trivial, but it's worth noting.

4. It would be more logical to use count() or sizeof() (same functions) in your "bHeadings" function instead of incrementing the head_count in the foreach:

PHP Code:
$this->head_count += count($table_headings);
// instead of: ++$this->heading_count; 
4. Utilize the === operand. It's an identical-comparison operand, it's actually a lot faster than == than you would expect. Use it wisely.

PHP Code:
// Since count() will always return an int, it's safe to use ===
if(count($row) === $this->heading_count
6. When you initialize your variables, set their values. There is no performance loss from this, and other languages actually benefit from it (not sure if PHP benefits from it, but it's still a good habit). This makes for good practice, and avoids possible type-confusion. For example:

PHP Code:
private $heading_count 0;
// Instead of private $heading_count;
// This way the type is instantly an integer, and not considered null, which is not an int. 
7. Use lower-case when typing keywords. There's actually a performance gain when doing this, although trivial, it's still worth noting. Also, in my opinion lowercase keywords are much prettier!

PHP Code:
        //**************************
        
$key null;                // looping key
        
$row null;                // looping value(is an array)
        
$key2 null;               // looping key
        
$val2 null;               // looping value
        //************************** 
8. Utilize the !== operand, it works the same way === does, regarding comparison.

PHP Code:
// Bad: if($this->error != NULL)
if($this->error !== null
----

Also, your singleton method.. I think it could use a touch-up, using __construct of course:

PHP Code:
    public static $CreateTable null;

    public function 
__construct()
    {
        
self::$CreateTable = &$this;
    }

    static public function 
getInstance()
    {
        if(
self::$CreateTable === null)
        {
            new 
self();
        }

        return 
self::$CreateTable;
    } 
I hope I've help a little.. :)

-Jay
Jay is offline  
Reply With Quote
Old 05-10-2009, 09:01 PM   #4 (permalink)
The Frequenter
Newcomer 
 
xenon's Avatar
 
Join Date: Dec 2007
Location: Bucharest, Romania
Posts: 438
Thanks: 3
xenon is on a distinguished road
Default

Quote:
Originally Posted by Jay View Post
1. Use brackets after the object name when creating new stuff: new object();
My question is: why? As long as the constructor doesn't take any parameters it's safe to use any variant. It's only a matter of preference.

Quote:
Originally Posted by Jay View Post
4. It would be more logical to use count() or sizeof() (same functions) in your "bHeadings" function instead of incrementing the head_count in the foreach: [...]
No, that would be a performance issue. You are talking about performance in variable names (which btw, I think it's bs, unless you give me a good reason for why your statement is true), but you can't see the difference in performance between calling a function and using a language construct?

Quote:
Originally Posted by Jay View Post
Also, your singleton method.. I think it could use a touch-up, using __construct of course: [...]
But that's the worst method that could be used and does not implement the singleton pattern (which says that only a limited number of instances should be allowed to be created from a class). With your implementation, nothing stops me from creating a thousand instances of the class (each of which will overwrite the static variable $CreateTable with the last instance created).

Here's the correct implementation:

PHP Code:
class Singleton
{
    protected static 
$Instance null;
    
    protected function 
__construct() { }
    
    public static function 
getInstance()
    {
        if(
self::$Instance === null)
        {
            
self::$Instance = new self();
        }
        
        return 
self::$Instance;
    }

__________________
I have optimistic thoughts, even though sometimes (if not always) life's a bitch.
xenon is offline  
Reply With Quote
Old 05-10-2009, 10:14 PM   #5 (permalink)
Jay
The Contributor
Good Samaritan 
 
Join Date: Dec 2007
Posts: 60
Thanks: 5
Jay is on a distinguished road
Default

Quote:
Originally Posted by xenon View Post
My question is: why? As long as the constructor doesn't take any parameters it's safe to use any variant. It's only a matter of preference.
Yep, purely preference.. Don't be lazy, just add the damn brackets and make it look formidable


Quote:
Originally Posted by xenon View Post
No, that would be a performance issue. You are talking about performance in variable names (which btw, I think it's bs, unless you give me a good reason for why your statement is true), but you can't see the difference in performance between calling a function and using a language construct?
I actually benchmarked this.. I had mixed thoughts about your comment, but I was right. If you benchmark it yourself (I did it over 1000 iterations with 12 objects in a test array), you will see that my method is actually a hair better. What makes my theory even more prominent is that he's incrementing a variable of a class (++$this->head_count is slower than ++$head_count).

Look at it this way: you're setting $this->head_count every time that foreach loops.. if it looped 1000 times, that variable would be set 1000 times, which is also 1000 mathematical calculations since he's incrementing it.. You can't honestly tell me that one function call seems like a bad idea. But then again, this argument is stupid -- both methods work, both methods are near equal in performance. There's nothing really to argue about I guess, it comes down to what you think is best. To get down to the nitty gritty, you would have to see how count() works internally.



Quote:
Originally Posted by xenon View Post
But that's the worst method that could be used and does not implement the singleton pattern (which says that only a limited number of instances should be allowed to be created from a class). With your implementation, nothing stops me from creating a thousand instances of the class (each of which will overwrite the static variable $CreateTable with the last instance created).
I'm speechless. I'm trying to help the guy, and you're mocking me. Instead of mocking, why don't you criticize? I'm sure people would actually respect your opinion if you didn't come on as so hostile.
Jay is offline  
Reply With Quote
Old 05-11-2009, 07:42 AM   #6 (permalink)
The Visitor
 
Join Date: May 2009
Posts: 2
Thanks: 0
frostyboy33 is on a distinguished road
Default

Hi guys thank you very much for the replies.
Have made me go back and change a couple of things.

in terms of actually usefulness do you guys think it is good?
and what could be done to either make it better or more "powerful"
frostyboy33 is offline  
Reply With Quote
Old 10-18-2012, 01:35 PM   #7 (permalink)
The Addict
 
Join Date: Oct 2012
Posts: 244
Thanks: 0
dashixiong is on a distinguished road
Default

Some conservatives have Coach Factory Outlet pushed that critique further, saying that Mr. Obama’s policies are too costly, often assist the wrong people Louis Vuitton Belts and could have the paradoxical effect of driving up college costs. The dispute turns not just on different Coach Factory Outlet assessments of how policies play out, but on differing philosophical views about the role of government. During Gucci Belts his time in office, Mr. Obama has sharply increased aid to low- and middle-income students, notably through the Pell Grant Coach Factory Outlet program, which grew from $14.6 billion given to 6 million students in 2008, to nearly $40 billion for Coach Factory Outlet almost 10 million students this year. His administration also made it easier to request aid, shortening the Coach Factory Online complex federal application and allowing people to transfer their financial information electronically from the Internal Coach Outlet Online Revenue Service database. But while many education experts laud his efforts, analysts of varying political Coach Outlet Online stripes have also questioned how much impact some of the president’s policies will have, noting that the prices Coach Online Outlet charged by colleges, and student borrowing, continue to climb.But behind the headlines about soaring costs, the Coach Factory Outlet Online reality is more complex and wildly uneven, because a growing number of students receive Coach Outlet Online financial aid, and only relatively high-income families pay those fast-rising sticker prices. Adjusted for Coach Factory Online inflation, the College Board calculates, the average net price changed little over the last decade at private Coach Factory Outlet schools, and rose only modestly at public ones.Defending federal spending, Arne Duncan, the secretary of Hermes Belts education, said that for more than 30 years, college prices had risen even when federal aid had not, leading him to believe Coach Factory Online there was zero correlation.
dashixiong is offline  
Reply With Quote
Old 10-22-2012, 09:12 AM   #8 (permalink)
The Addict
 
Join Date: Oct 2012
Posts: 244
Thanks: 0
dashixiong is on a distinguished road
Default Coach Outlet

You’ve relativelyCoach Outlet recently arrived in New Delhi after living in two of Asia’s other great cities,Coach Outlet Store Online Tokyo and Hong Kong, for several years. Do these cities feel like they’re part of the same continent? Yes, and no. In terms Coach Factory Onlineof infrastructure, they couldn’t be more different. Getting regularCoach Outlet power and water at my house in New Delhi is never a sure thing, even though Coach Purse Outlet OnlineI’m paying the same rent that I paid in Tokyo and almost the same electricity prices. Both Hong Kong and Tokyo are also crowded places,Coach Factory Outlet Online but both cities are incredibly well planned and efficiently run. Efficient is not a word I would use to describe my Coach Bags Outlet Onlineday-to-day life in New Delhi. On the other hand, one thing that I think Hong Kong and New Delhi have in common isCoach Handbags Outlet a shared sense of optimism — a feeling that the best is yet to come. That’s definitely not the feeling you get in Tokyo,Coach Outlet Online or in the U.S. when I go home. It’s a big part of what I find addictive about living and working in this part of the world. You feel like you’re watching the future unfold.
dashixiong is offline  
Reply With Quote
Reply



Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On

Similar Threads
Thread Thread Starter Forum Replies Last Post
[Tutorial] How to organize your classes | Part 1 Tanax Advanced PHP Programming 10 03-01-2009 10:08 PM
A Generic Singleton Base Class Theo Advanced PHP Programming 7 08-18-2008 02:25 AM
[Tutorial] Basic tutorial about class basics Tanax Absolute Beginners 14 07-24-2008 01:37 PM
PHP5 Classes A to Z Part 1 quantumkangaroo Advanced PHP Programming 11 04-01-2008 04:21 AM
Tutorial: PHP and OOP, a beginners guide Village Idiot Tips & Tricks 0 09-06-2007 04:23 PM


All times are GMT. The time now is 08:22 PM.

 
     

Powered by vBulletin® Version 3.6.8
Copyright ©2000 - 2013, Jelsoft Enterprises Ltd.
Search Engine Optimization by vBSEO 3.1.0
Inactive Reminders By Icora Web Design