viki00762 Posted June 6, 2017 Share Posted June 6, 2017 Hey everyone, Help an Intern out please. I am trying to call a function from another function in a class but I am not successful. I am trying to call ToDbDateTime from GetVisitors, Please provide your inputs. Code as follows:- <?php class Visitor{ public $website = -1; public $first_visit; public $last_visit; public $google_cid; public $pfcount; public $pgoodclickcount; public $clickfraudexpert = "Unknown"; private static function map_pdo($pdo) { $object = new Visitor(); $object->recnum = $pdo->recnum; $object->date_created = self::FromDbDateTime($pdo->date_created); $object->date_updated = self::FromDbDateTime($pdo->date_updated); $object->website = $pdo->website; $object->pfcount = $pdo->pfcount; $object->pgoodclickcount = $pdo->pgoodclickcount; $object->clickfraudexpert = $pdo->clickfraudexpert; $object->first_visit = self::FromDbDateTime($pdo->first_visit); $object->last_visit = self::FromDbDateTime($pdo->last_visit); $object->google_cid = $pdo->google_cid; //$object->meta = self::DecodeMeta($pdo->json, new FsVisitorMeta()); return $object; } public static function ToDbTime($date){ $str = null; if($date!=null) { $dt = clone $date; $dt->setTimezone(new \DateTimeZone('UTC')); $str = $dt->format('H:i:s'); } return $str; } public static function ToDbDateTime($date) // Function I have to call { $str = null; if($date!=null) { $dt = clone $date; //Error here for non Object $dt->setTimezone(new \DateTimeZone('UTC')); $str = $dt->format('Y-m-d H:i:s'); } return $str; } public static function GetVisitors($website,$start=null, $end=null, $date_range=null) { $host = "localhost"; $username = "user"; $password = "pwdy"; $dbname = "crm"; $charset = 'utf8'; $opt = [ PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, PDO::ATTR_EMULATE_PREPARES => false, ]; $dsn = "mysql:host=$host;dbname=$dbname;charset=$charset"; try{ $pdo = new PDO($dsn, $username, $password, $opt); $pdo->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION ); //Last 7 Days Data if($date_range==null){ $data = array('website' => $website); echo "<br>Null"; $data['start_time'] = self::ToDbDateTime($start);// Call to function $stmt = $pdo->prepare('SELECT recnum, date_created ,date_updated,website ,first_visit ,last_visit ,google_cid ,pfcount ,clickfraudexpert,pgoodclickcount FROM visitors WHERE website=:website and last_visit >= convert(:start,date) and last_visit <= convert(:end,date) order by last_visit desc limit 1'); //$stmt->execute(['website' => $website]); $stmt->execute([$website, $start, $end]); while($row=$stmt->fetch(PDO::FETCH_ASSOC)) { //print_r($result); echo "<br>" . "recnum: " . $row["recnum"] . "<br>" . "date_created:" . $row["date_created"] . "<br>" . "date_updated:" . $row["date_updated"] . "<br>" . "website :" . $row["website"]. "<br>". "last_visit: " . $row["last_visit"] . "<br>" . "first_visit:". $row["first_visit"] . "<br>" . "google_cid :" . $row["google_cid"]."<br>" . "pfcount:". $row["pfcount"]. "<br>" . "pgoodclickcount:". $row["pgoodclickcount"]. "<br>" . "clickfraudexpert:". $row["clickfraudexpert"]. "<br>"; } } } catch (PDOException $e) { echo $e->getMessage(); } } public static function main(){ ini_set('display_errors', 1); //map_pdo(); $call = new Visitor(); $call->GetVisitors("160",'2017-05-22', '2017-06-01', ''); //Give start date and end date for manual range and text for rpe set data ranges } } Visitor::main(); ?> The code throwing the error as '__clone method called on non-object in '$dt = clone $date;' Please help me with this and also I need to take the output out of the function, I need your inputs on that as well. Thanks Quote Link to comment Share on other sites More sharing options...
Psycho Posted June 6, 2017 Share Posted June 6, 2017 $this->ToDbDateTime($start); Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 6, 2017 Share Posted June 6, 2017 What a mess -- and I mean both that wall of unformatted plaintext you've posted and the code itself. Looks like you've been using your Visitor class as some kind of code dump where you just put everything which is needed, ranging from date conversions to connecting to the database (once per query -- WTF?). It even has its own main() method like it was an application by itself. Don't do this. A class should do one thing and one thing only. It's not the job of a visitor to convert dates or establish database connections. It may use those features, but not implement them. The implementation belongs into other classes which again only do one thing. If you keep your classes simple and well-organized, your error rate will immediately drop. Then you have no validation besides a few null checks. Your methods accept anything, and if the input is invalid, you're doing nonsense like trying to clone a string. Last but not least, you have conceptual problems. Your data conversion methods all assume you're passing DateTime objects to them, but you're obviously dealing with date strings instead. Or maybe that was just your test. We'll never know, because there's no documentation either. Time to clean up the mess: Remove everything from the class which isn't visitor-related. Make the methods type-safe and validate the arguments, so that errors are caught as early as possible. Document the methods. Even if you're the only one working on the code, this is very helpful for ordering your thoughts. 1 Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 6, 2017 Share Posted June 6, 2017 $this->ToDbDateTime($start); There is no $this in a static context. Quote Link to comment Share on other sites More sharing options...
viki00762 Posted June 7, 2017 Author Share Posted June 7, 2017 @Guru Thanks for the invaluable inputs. I just started working on PHP so i think i am failing to implement the PHP concepts. Yes i am the only one writing this code. Basically I have get the last 7 days data from the DB and dump it on the browser. I have modified my code based on your recommendations segregating the connection to DB to organize the classes but i am having hard time connecting to my DB and fetching result like this. <?php namespace FsCRM { require 'C:\xampp\htdocs\testing\FsFrameworkController.php'; require 'C:\xampp\htdocs\testing\FsStorableObject.php'; class FsVisitor extends FsStorableObject { public $website = -1; public $first_visit; public $last_visit; public $google_cid; public $pfcount; public $pgoodclickcount; public $clickfraudexpert = "Unknown"; public function Store() { try { $sql = ""; $data = null; if ($this->recnum != -1) { $data = array( 'recnum' => $this->recnum, 'website' => $this->website, 'pfcount' => $this->pfcount, 'pgoodclickcount' => $this->pgoodclickcount, 'clickfraudexpert' => $this->clickfraudexpert, 'first_visit' => self::ToDbDateTime($this->first_visit), 'last_visit' => self::ToDbDateTime($this->last_visit), 'google_cid' => $this->google_cid ); $sql = "UPDATE visitors SET date_updated=UTC_TIMESTAMP(), website=:website, pfcount=:pfcount, pgoodclickcount=:pgoodclickcount, clickfraudexpert=:clickfraudexpert, first_visit=:first_visit, last_visit=:last_visit, google_cid=:google_cid WHERE recnum=:recnum;"; } else { $data = array( 'website' => $this->website, 'pfcount' => $this->pfcount, 'pgoodclickcount' => $this->pgoodclickcount, 'clickfraudexpert' => $this->clickfraudexpert, 'first_visit' => self::ToDbDateTime($this->first_visit), 'last_visit' => self::ToDbDateTime($this->last_visit), 'google_cid' => $this->google_cid ); $sql = "INSERT INTO visitors (date_created, website, pfcount, pgoodclickcount, clickfraudexpert, first_visit, last_visit, google_cid) VALUES(UTC_TIMESTAMP(), :website, :pfcount, :pgoodclickcount, :clickfraudexpert, :first_visit, :last_visit, :google_cid);"; } $framework_controller = FsFrameworkController::GetInstance(); $framework_controller->data_model->ExecuteAction($sql, $data); if ($this->recnum == -1) { $this->recnum = $framework_controller->data_model->GetLastInsertId(); } } catch(\Exception $e) { FsFrameworkController::LogException($e); } return $this->recnum; } // public function Delete() // { // try // { // $data = array('recnum' => $this->recnum); // $sql = "DELETE from visitors WHERE recnum=:recnum;"; // $framework_controller = FsFrameworkController::GetInstance(); // $framework_controller->data_model->ExecuteAction($sql, $data); // } // catch(\Exception $e) // { // FsFrameworkController::LogException($e); // } // } //PDO private static function map_pdo($pdo) { $object = new FsVisitor(); $object->recnum = $pdo->recnum; $object->date_created = self::FromDbDateTime($pdo->date_created); $object->date_updated = self::FromDbDateTime($pdo->date_updated); $object->website = $pdo->website; $object->pfcount = $pdo->pfcount; $object->pgoodclickcount = $pdo->pgoodclickcount; $object->clickfraudexpert = $pdo->clickfraudexpert; $object->first_visit = self::FromDbDateTime($pdo->first_visit); $object->last_visit = self::FromDbDateTime($pdo->last_visit); $object->google_cid = $pdo->google_cid; //$object->meta = self::DecodeMeta($pdo->json, new FsVisitorMeta()); return $object; } // Getting only record number public static function GetVisitor($recnum) { $object = null; try { $data = array('recnum' => $recnum); $sql = 'SELECT * FROM visitors WHERE recnum=:recnum;'; $framework_controller = FsFrameworkController::GetInstance(); $query = $framework_controller->data_model->ExecuteReader($sql, $data); $row = $query->fetch(); if($row) { $object = self::map_pdo($row); } } catch(\Exception $e) { FsFrameworkController::LogException($e); } return $object; } //Getting Count public static function GetVisitorsCount($website, $start=null, $end=null) { $count = 0; try { $data = array('website' => $website); $sql = 'SELECT count(recnum) as visitors_count FROM visitors WHERE website=:website'; if($start != null) { $data['start_time'] = self::ToDbDateTime($start); $sql .= ' AND last_visit>=:start_time'; } if($end != null) { $data['end_time'] = self::ToDbDateTime($end); $sql .= ' AND last_visit<=:end_time'; } $sql .= ';'; $framework_controller = FsFrameworkController::GetInstance(); $query = $framework_controller->data_model->ExecuteReader($sql, $data); while($row = $query->fetch()) { $count = $row->visitors_count; } } catch(\Exception $e) { FsFrameworkController::LogException($e); } return $count; } // Pull Data public static function GetVisitors($website, $start=null, $end=null, $limit_start=null, $limit_length=null, $sort_by=null, $sort_direction=null) { $objects = array(); try { $data = array('website' => $website); $sql = 'SELECT * FROM visitors WHERE website=:website'; if($start != null) { $data['start_time'] = self::ToDbDateTime($start); $sql .= ' AND last_visit>=:start_time'; } if($end != null) { $data['end_time'] = self::ToDbDateTime($end); $sql .= ' AND last_visit<=:end_time'; } if($sort_by != null && strlen($sort_by)>0 && $sort_direction != null && strlen($sort_direction)>0) { $sql .= ' ORDER BY '.$sort_by.' '.$sort_direction; } else { $sql .= ' ORDER BY last_visit DESC'; } if($limit_start != null && $limit_length != null) { $sql .= " LIMIT ".$limit_start.", ".$limit_length; } $sql .= ';'; var_dump($data); $framework_controller = FsFrameworkController::GetInstance(); //Erro on Next Line $query = $framework_controller->data_model->ExecuteReader($sql, $data); while($row = $query->fetch()) { $objects[] = self::map_pdo($row); } } catch(\Exception $e) { FsFrameworkController::LogException($e); } return $objects; } public function SetMeasurementProtocolData($ga_data=array()) { if(strlen($this->google_cid)>0) { $ga_data['cid'] = $this->google_cid; } return $ga_data; } } ini_set('display errors', 1); error_reporting(E_ALL); $call = new FsVisitor(); $call->GetVisitors("160",'2017-05-22 15:22:24', '2017-06-01', '1', '1', 'last_visit', 'desc'); } ?> Its throwing error as 'Fatal error: Call to a member function ExecuteReader() on null on Line 209' Please help me figure this out. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 7, 2017 Share Posted June 7, 2017 I think the error message is pretty clear: You're trying to call the ExecuteReader() method of $framework_controller->data_model, but $framework_controller->data_model is null. If you want to know why it's null, then you have to look at your FsFrameworkController class. You haven't posted the code, so we cannot tell you. But why make it so complicated? Just create one PDO connection when the application is initialized, pass it to your data classes via the constructor and put it into an attribute for later use. No need for singleton gymnastics. <?php class SomeDataClass { /** * @var PDO the underlying database connection */ private $databaseConnection; /** * The constructor * * @param PDO $databaseConnection the underlying database connection */ public function __construct(PDO $databaseConnection) { if (is_null($databaseConnection)) { throw new InvalidArgumentException('Database connection must not be null.'); } $this->databaseConnection = $databaseConnection; } /** * A test function to demonstrate the use of the database connection * * @return int a test value */ public function testQuery() { return $this->databaseConnection->query('SELECT 123')->fetchColumn(); } } And you still haven't added any validations. Even worse: Now you're actively suppressing exceptions and breaking the error checks which are already there. Don't do that. I'm not sure why PHP programmers have this strange urge to catch exceptions, but this is stupid. Exceptions exist for a reason. They mean something went very wrong. Unless you have an actual solution to the problem (which is rather unlikely in the case of a failed query), just leave the exception alone. Let PHP abort the script in a controlled manner. I really think you should learn the basics before you start producing large amounts of code. When you don't really know what you're doing, you'll have to rewrite the code over and over again. 1 Quote Link to comment Share on other sites More sharing options...
viki00762 Posted June 7, 2017 Author Share Posted June 7, 2017 Thanks a lot for pointing out the errors. Once I will be able to get the data from the DB, I will perform validations and other error checks. In the above code, I am taking care of the Null Checks as well. I did not understood 'breaking the error checks which are already there', if you may elaborate on this please pointing the line no. Thanks GURU P.S:- I am not a PHP programmer yet Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 7, 2017 Share Posted June 7, 2017 I did not understood 'breaking the error checks which are already there', if you may elaborate on this please pointing the line no. I'm talking about all of your try statements. You catch exceptions without even knowing what kind of error you're dealing with, you log the message, and then you continue the application. This not only defeats the whole purpose of exceptions. It's actually dangerous, because the error may be very severe and have put the application into an unstable state. In this case, you definitely do not want to continue like nothing happened. As I already said, leave the exceptions alone. PHP will abort the script in a controlled manner and send the message to the screen or a logfile (depending on the configuration). Catching an exception only makes sense if you can actually solve the problem or have a special handling procedure -- which is very rare. Most of the time, the error should be handled by PHP. Quote Link to comment Share on other sites More sharing options...
viki00762 Posted June 8, 2017 Author Share Posted June 8, 2017 Yes I implemented that and PHP is aborting the script in a controlled manner, thanks for the recommendation. Although, I am able to print the data n the function but on returning the objects to the called function (GetVisitors), I am not able to print the dates. Its throwing Error as 'Catchable fatal error: Object of class DateTime could not be converted to string ' //This is my ToDbDateTime function which is called at Line 177 to convert into date format public static function ToDbDateTime($date) { $str = null; if($date!=null) { //$dt = clone $date; $dt = new \DateTime($date); $dt->setTimezone(new \DateTimeZone('UTC')); $str = $dt->format('Y-m-d H:i:s'); } return $str; } And I am printing the dates as: $visitors = $call->GetVisitors("160",'2017-05-22',null,'0','10','last_visit','desc', null); if(is_array($visitors) && count($visitors>0)){ foreach($visitors as $resultset){ echo "<br>" . "recnum: " . $resultset->last_visit . "</td>" . "</tr>"; } } Please help on how to dump the date column values. Thanks Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 8, 2017 Share Posted June 8, 2017 Your visitors method is calling FromDbDateTime(), because the date is coming from the database. This method obviously turns the date strings into DateTime objects. You cannot print a DateTime, because PHP has no idea how the result is supposed to look like. There are many, many different formats in use today, so you have to say explicitly what you want. Proper documentation and a good IDE would have prevented this. When you document the types of your attributes, some IDEs (like PhpStorm) can validate your code while you write it and immediately tell you when something is wrong. For example: As you can see, the IDE now knows the types of the attributes and methods. When you make a mistake, you immediately get a notice -- which in this case is even better than the PHP error message. 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.