 |
Account Login
|
 |
 |
Latest Articles
|
 |
 |
IRC Channel
|
 |
 |
Associates
|
 |
 |
Associates
|
 |
|
 |
 |
|
 |
05-08-2009, 10:20 PM
|
#1 (permalink)
|
|
The Visitor
Join Date: May 2009
Posts: 2
Thanks: 0
|
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.
|
|
|
|
05-09-2009, 09:58 AM
|
#2 (permalink)
|
|
The Prestige
Join Date: Sep 2007
Location: Sweden, Stockholm
Posts: 1,080
Thanks: 115
|
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 
__________________
|
|
|
|
05-10-2009, 07:39 PM
|
#3 (permalink)
|
|
The Contributor
Join Date: Dec 2007
Posts: 60
Thanks: 5
|
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
|
|
|
|
05-10-2009, 09:01 PM
|
#4 (permalink)
|
|
The Frequenter
Join Date: Dec 2007
Location: Bucharest, Romania
Posts: 438
Thanks: 3
|
Quote:
Originally Posted by Jay
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
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
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.
|
|
|
|
05-10-2009, 10:14 PM
|
#5 (permalink)
|
|
The Contributor
Join Date: Dec 2007
Posts: 60
Thanks: 5
|
Quote:
Originally Posted by xenon
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
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
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.
|
|
|
|
05-11-2009, 07:42 AM
|
#6 (permalink)
|
|
The Visitor
Join Date: May 2009
Posts: 2
Thanks: 0
|
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"
|
|
|
|
10-18-2012, 01:35 PM
|
#7 (permalink)
|
|
The Addict
Join Date: Oct 2012
Posts: 244
Thanks: 0
|
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.
|
|
|
|
10-22-2012, 09:12 AM
|
#8 (permalink)
|
|
The Addict
Join Date: Oct 2012
Posts: 244
Thanks: 0
|
Coach Outlet
You’ve relatively Coach 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 regular Coach 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 is Coach 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.
|
|
|
|
|
Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
|
|
|
| Thread Tools |
Search this Thread |
|
|
|
| Display Modes |
Linear Mode
|
Posting Rules
|
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
HTML code is Off
|
|
|
|