| SitePoint Sponsor |
Hey there i am just starting to try pdo in OO. I am getting this error Call to a member function prepare() on a non-object
I have a constructor
and then i am calling the prepare like soPHP Code:public function __construct($pdo){
$this->pdo = $pdo;
}
for those of you that work with pdo in OO how do you set it up. Am i on the right track?PHP Code:$this->pdo->prepare($query)
Thanks in advanced!
You may need to show a bit more code than that.
We cannot tell if there is a problem with the class you seem to have created whose constructor you are showing.
Do you have PDO working correctly on its own, outside of this class?
Ok.Originally Posted by Cups
Yes it is working correctly outside of the class.
here's the full concturctor for the classHere is the login functionPHP Code:public function __construct($pdo,$clean){
$this->pdo = $pdo;
$this->clean = $clean;
}
PHP Code:public function login($email,$rawPass){
$this->email = $this->clean->text($email);
$this->rawPass = $this->clean->text($rawPass);
$loginSelectQuery = "SELECT COUNT(*) AS num_rows FROM users WHERE email=:email";
$values = array('email' => $this->email);
try {
$checkEmail = $this->pdo->prepare($query);
$checkEmail->execute($values);
} catch (PDOException $e) {
$title .= "Error";
$error->mysql();
}
$check = $checkEmail->fetch(PDO::OBJ);
if($check->num_rows == 1){
$query = "SELECT * FROM users WHERE email = :email";
try {
$getUserDetails = $this->pdo->prepare($query);
$getUserDetails->execute($values);
} catch (PDOException $e) {
$title .= "Error";
$error->mysql();
}
$getUserDetails->fetch(PDO::OBJ);
$password = hash('sha256',$this->rawPass).$getUserDetails->hash;
if($password == $getUserDetails->password){
$_SESSION['name'] = $getUserDetails->name;
$_SESSION['id'] = $getUserDetails->id;
$_SESSION['farmName'] = $getUserDetails->farmName;
$_SESSION['authLevel'] = $getUserDetails->level;
$title .= "Logged in!";
$referer = $_SERVER['HTTP_REFERER'];
header('Refresh: 3, url='.$referer);
$output .= "You have been logged in as {$_SESSION['name']}. You well be redirected to the previous page within five(5) seconds";
}
else {
$title .="Wrong Password";
$output .= "The password you entred was not correct";
}
}
else{
$tile .= "User does not exist";
$output .= "Hello there. We could not locate the email <strong>{$this->email}</strong> in our database";
}
}
You're doing:
But your query variable is actually called $loginSelectQuery.PHP Code:$checkEmail = $this->pdo->prepare($query);
Also, I'm pretty sure you're missing a colon from the 'email' array key for the $values array:
Think it should be this:
Try that?PHP Code:$values = array(':email'=>$this->email);
I think these instances are incorrect:
SBPHP Code:$checkEmail->fetch(PDO::OBJ);
PHP Code:$checkEmail->fetch(PDO::FETCH_OBJ);
Yes Cups that was it. So now it's executing fine. i just have to figure out the sessions are not being set.
if i use return on a variable that well said var. to be displayed on a page? Correct?
Yes Cups that was it. So now it's executing fine. i just have to figure out the sessions are not being set.
if i use return on a variable that well said var. to be displayed on a page? Correct?
Probably because you are not stating session_start() anywhere ... that I can see anyhow.
Overall this login method (never mind the class) is taking on an awful lot of responsibility.
A class ought to do one single thing, and do it properly.
It is OK to have a class which does very little but tells lots of other smaller classes what to do, delegating and orchestrating if you will.
Here are some pseudocode-ish ideas, not to strip your class down to one single thing, but to achieve a bit of a halfway house, so you should still recognise what it does - but also appreciate how some of the complexity and rigidity can be stripped out.
I am hinting at the existence of a simple Session class which does not yet exist, but is simple enough to code up (never mind find on the web somewhere)PHP Code:class LoginValidator {
function __constructor(PDO $PDO ){
// set up your pdo as a member (property)
}
function checkCredentials($user ='', $password=''){
// just do one single select here ;
$query = "SELECT id, name, farmname, level FROM users WHERE email=:email AND password = :password";
on success -- tell $this->startSession($session_vars) to start the session
return true;
else (on fail)
return false;
}
private function startSession($session_vars){
// this imagines using another class which serves to be used in many places in your code
// either pass an array of values as an argument
// or access private members which checkCredentials() would set up for you
$s = new Session($session_vars);
}
}
// usage -- where $pdo is created elsewhere, it is already set up, you just pass it as an argument...
if( user and password are set ) {
$lv = new LoginValidator($pdo);
}
if( $lv->checkCredentials($_POST['user'], $_POST['password']) is false ){
NOW redirect the user to somewhere else
} else {
redirect the user back to the page
}
Going back to the CheckCredentials class idea above (I have no idea what your class is called) even this name CheckCredentials is not accurate because as I have written it it is more accurately a CheckCredentialsAndStartSession class (I remember calling a similar class "Bouncer" -- as in a doorman)
But still, it is now orchestrating, delegating to other classes what to do.
It does one basic thing (sort of), it checks to see if an instance of user and pass exist in the db
It delegates to a Session class
It is given an instance of a PDO which it needs to do its job (it should not care whether that is using a mysql or sqlite driver)
It leaves the responsibility of redirecting the user to the userland coder, though this could be another class too ...
You will notice I have completely ignored your clean() method, I can only guess what that does and surmise that it is not adding anything to the party at all.
Notice it does NOT check whether user and pass are even set, it is pretty dumb in that respect. You could smarten it up, you could start adding an array of different error messages ... but they would differ depending on which application was using it -- the downside would be that it would start cluttering up your LoginValidator.
Now the trick is, can you alter LoginValidator slightly so that it is not forever tied to your users/farm application?
Then you can use it, like the Session class, in many more of your development tasks.
My example is not meant to be perfect by any means, just some ideas to get you thinking, if some of this makes sense to you and interests you then it could be time to do some OOP study.
Probably because you are not stating session_start() anywhere ... that I can see anyhow.
Overall this login method (never mind the class) is taking on an awful lot of responsibility.
A class ought to do one single thing, and do it properly.
It is OK to have a class which does very little but tells lots of other smaller classes what to do, delegating and orchestrating if you will.
Here are some pseudocode-ish ideas, not to strip your class down to one single thing, but to achieve a bit of a halfway house, so you should still recognise what it does - but also appreciate how some of the complexity and rigidity can be stripped out.
I am hinting at the existence of a simple Session class which does not yet exist, but is simple enough to code up (never mind find on the web somewhere)PHP Code:class LoginValidator {
function __constructor(PDO $PDO ){
// set up your pdo as a member (property)
}
function checkCredentials($user ='', $password=''){
// just do one single select here ;
$query = "SELECT id, name, farmname, level FROM users WHERE email=:email AND password = :password";
on success -- tell $this->startSession($session_vars) to start the session
return true;
else (on fail)
return false;
}
private function startSession($session_vars){
// this imagines using another class which serves to be used in many places in your code
// either pass an array of values as an argument
// or access private members which checkCredentials() would set up for you
$s = new Session($session_vars);
}
}
// usage -- where $pdo is created elsewhere, it is already set up, you just pass it as an argument...
if( user and password are set ) {
$lv = new LoginValidator($pdo);
}
if( $lv->checkCredentials($_POST['user'], $_POST['password']) is false ){
NOW redirect the user to somewhere else
} else {
redirect the user back to the page
}
Going back to the CheckCredentials class idea above (I have no idea what your class is called) even this name CheckCredentials is not accurate because as I have written it it is more accurately a CheckCredentialsAndStartSession class (I remember calling a similar class "Bouncer" -- as in a doorman
But still, it is now orchestrating, delegating to other classes what to do.
It does one basic thing (sort of), it checks to see if an instance of user and pass exist in the db
It delegates to a Session class
It is given an instance of a PDO which it needs to do its job (it should not care whether that is using a mysql or sqlite driver)
It leaves the responsibility of redirecting the user to the userland coder, though this could be another class too ...
You will notice I have completely ignored your clean() method, I can only guess what that does and surmise that it is not adding anything to the party at all.
Notice it does NOT check whether user and pass are even set, it is pretty dumb in that respect. You could smarten it up, you could start adding an array of different error messages ... but they would differ depending on which application was using it -- the downside would be that it would start cluttering up your LoginValidator.
Now the trick is, can you alter LoginValidator slightly so that it is not forever tied to your users/farm application?
Then you can use it, like the Session class, in many more of your development tasks.
My example is not meant to be perfect by any means, just some ideas to get you thinking, if some of this makes sense to you and interests you then it could be time to do some OOP study.
Some good comments cups...
I was thinking, perhaps it might make sense to have the session class completely outside of this class, and in the calling code have something like:
Something along those lines anyway...PHP Code:
$loginValidator = new LoginValidator($pdoObject);
$sessionObject = new SessionObject();
if ($loginValidator->checkCredentials($username, $password)){
$sessionObject->login($username);
}
Actually having thought about it more, perhaps you could have a user object, which the loginValidator could return if the user was successfully logged in, so it could go something like this:
Then from there you could grab the user object out of the session with something like:PHP Code:
$loginValidator = new LoginValidator($pdoObject);
$sessionObject = new SessionObject();
if ($loginValidator->checkCredentials($username, $password)){
$sessionObject->setUserObject($loginValidator->getLoggedInUser());
}
$sessionObject->getLoggedInUser();
And could then call methods on the user object such as:
Etc... what do you reckon?PHP Code:$userObject = $sessionObject->getLoggedInUser();
echo $userObject->getUsername();
I mean, yeah, now we're off ... thanks for replying, it underlines and supports a point I did not make very clear, that there are many ways of doing the same thing.
If it works for you then great. We could now go off and play with trade-offs between responsibilities, degrees of coupling and will no doubt feature factories vs Dependency Injection and all the OOP buzzwords.
This is all grist to the OOP mill, and I am happy to discuss for as far as my Pattern knowledge will carry me (which is NOT as advanced as many on here may think).
The thing is, I don't want to leave the OP behind when it is clear from the method example posted there are some fundamental OOP principles that are being broken and clearly the OP could do with some help to shed some light on what is going on with the code.
I don't want to come across as being pretentious here but even if it we can have the OP think (or anyone else watching -- why not say hello?), "Crikey, there is a bit more to this OOP lark than I first thought" and that he/she would come back and ask more questions about objects then that would be really, really great.
Because, you know what, some of the old OOP Gods that used to hang out on here might come back and share some of their wisdom.
I don't know about you, but when I start thinking about OOP, I find it helpful to imagine I am talking to my dogs.
They are fantastic at doing one thing.
Eat dinner.
Chase a stick.
Chase a rabbit.
if (you see another dog) growl.
if (I call you) come back here.
But if I tell them to "bring back the stick, and if you do not growl at that dog, and you can eat 50% of your dinner" -- then it can go wrong in so many ways I never dreamed of.
(off to walk them now as it happens...)
Ciao
Ok. So let me see if i get this.
It is better (when working with OO style) to break things down in to there own functions in side a class. Say if i was to make a document function and i wanted to validate that the title and document field could i use one class to validate both the title and document and then if both are valid have a variable returned as true then use that in a different class.
example
is that more of the way it should be done?? Also this may be a basic question, but do i have to putPHP Code:<?php
class documents{
public function __construct($pdo,$clean,$document){
$this->pdo = $pdo;
$this->clean = $clean;
$this->document = $document
}
public function charCount(){
$this->chars = substr($this->document,0);
return $this->chars;
}
public function validate($title,$document){
$this->title = $title;
$this->document = $document;
if(empty($this->title) || $this->title == 0){
$title .= "Empty feild";
$output .= "The title is empty. Please fill in the title";
$this->error['title'] = TRUE; }
if($this->chars === 0){
$title .= "Document Empty";
$output .= "We can't save the document! There are no characters in it! We can only save documents with words in them!";
$this->error['docChars'] = TRUE;
}
if(!$this->error['title'] && !$this->error['docChars'] == TRUE){
$this->validated = TRUE;
return $this->validated;
}
}
publixc function insert(){
if($this->validated == TRUE){
$query = "blah"
}
}
}in the function. Or can i execlude and well it still work. Or what is recommended?PHP Code:$this->charCount
A class should do one thing properly. It can have as many moving parts as you want (methods). You can have your class call its own methods.It is better (when working with OO style) to break things down in to there own functions in side a class.
Your documents class does not seem to do much at all, it checks that the title and the document themselves are not empty, but lets stay with it and look at a few of the options you could take into consideration.
Perhaps it would help to think about how you'd like to initiate and call the class methods from user land:
a) You could do this:PHP Code:<?php
// presumably this is coming from a HTTP request ...
$document = "Incoming data forming the document"';
$title = "The document Title";
// $pdo this has been instantiated somewhere else, and is perhaps included
OR b) you could design it to react to this:PHP Code:// go ahead and instantiate your class
$storer = new DocumentStorer($pdo);
if( $storer->checkNotNull($title) && $storer->checkNotNull($document) ){
$storer->save( $document, $title);
}else{
$storer->getMessages();
}
One difference is that in a) you leave the option open to store a document without a Title, or, in the future you could add a new field, Author, without having to go back and change the insides of your DocumentStorer by just doing $storer->checkNotNull($author);PHP Code:// go ahead and instantiate your class
$storer = new DocumentStorer($pdo);
if( $storer->save($document, $title) === false ){
echo $storer->getMessages();
}
Some might thing this is academic, e.g. why would you even want to instantiate a class when a simple empty() check on the incoming variables prior to starting, but we will leave that aside for the sake of illustrating my points.
Now you could also do c) which is what I think you are trying to do, the big single bang.
How you would build your class would then depend on whether you want to do a) b) or c) (or d, e, f for that matter).PHP Code:// feed the class everything it needs in one go
$storer = new DocumentStorer($pdo, $document, $title);
// I don't much care how you do it, either be successful or get an error message back
if( $storer->save() === false ){
echo $storer->getMessages();
}
The code to satisfy c) would look something like this, I put some echo statements in there so you can see what is happening, you can add getMessages() or a getErrors() method yourself.
PHP Code:class DocumentStorer {
public $pdo, $title, $doc;
public $errors = array();
function __construct($pdo, $doc, $title){
$this->pdo = $pdo;
$this->title = $title;
$this->doc = $doc;
}
function save(){
echo 'doing save attempt.... ';
$this->checkNotNull($this->doc, 'Document');
$this->checkNotNull($this->title, 'Title');
if( count($this->errors) > 0) {
echo 'Fail ...';
return false;
}else{
// this is where you'd save your data or delegate its storing
echo "Saving $this->doc with $this->title using $this->pdo";
return true;
}
}
function checkNotNull($var, $type){
echo 'doing not null check .... ';
if( strlen(trim($var)) === 0) {
$this->errors[] = "$type has no contents!";
}
}
}
$doc = "this is a doc";
$title = "this is a title";
$pdo = "PDO instance";
$storer = new DocumentStorer($pdo, $doc, $title);
if( !$storer->save() )
var_dump( $storer->errors);
Thanks! You guys (And gals?) on SPF are the best! Now i like the idea of a, checking to see if it is null with checkNotNull and think it would be way easier to add stuff in the future if i needed to.
Can any one recomend a OO php book. I have a few from SP but would deffently like to read more.
Would you do some thing like this ($name would be to set the error array and some thing to return to the users if empty)
Or how would you do it. I just want to learn. I appreciate your help so much!!PHP Code:public function checkNotNull($var,$name){
$this->var = $var;
$this->name = $name;
if(empty($this->var) || $this->var == 0){
$title .= "Empty feilds";
$output .= "The $this->name is empty. Please fill in the $this->name feild";
$this->error[$this->name] = TRUE;
}
}
if(!$this->error[$this->name] == TRUE){
$this->validated = TRUE;
return $this->validated;
}
}
Well, a guy here actually. You are free to rename the methods to something that means more to you if you want, I just found it easier to rewrite what you had as a way of illustrating what could be done than to line by line tell you where I think you were going wrong.
There are just so many ways of approaching your particular problems, but yes, your reply "... think it would be way easier to add stuff in the future if i needed to." makes me very happy
There have been lots of threads about OOP books if you search this forum, but it'd be interesting to see if there are any new ones.
I have not read them all, so have nothing to add to those old forum posts you should find.
Posting Permissions
Reply With Quote
Bookmarks