CODEWITHSUNDEEP
phpoopclass
Cory Dee
Cory Dee

Reputation: 2891

Class Instantiation Fails

I'm having some problems getting my object to gracefully fail out if an invalid parameter is given during instantiation. I have a feeling it's a small syntax thing somewhere that I simply need fresh eyes on. Any help is more than appreciated.

class bib {
   protected $bibid;
   public function __construct($new_bibid) {
      if(!$this->bibid = $this->validate_bibid($new_bibid)) {
        echo 'me';
        return false;
      }
      //carry on with other stuff if valid bibid
    }

    private static function validate_bibid($test_bibid) {
       //call this method every time you get a bibid from the user
       if(!is_int($test_bibid)) {
            return false;
       }
       return (int)$test_bibid;
    }
 }

Note that I have an 'echo me' line in there to demonstrate that it is in fact, returning false. The way that I'm calling this in my PHP is as follows:

if(!$bib=new bib('612e436')) {
    echo 'Error Message';
} else {
    //do what I intend to do with the object
}

This outputs the me from above, but then continues on into the else block, doing what I intend to do with a valid object.

Can anyone spot anything I'm doing wrong in there?

Thanks!

Upvotes: 1

Views: 378

Answers (3)

dusoft
dusoft

Reputation: 11479

definitely, you need to have comparison with == instead of = which is an assign operation.

Upvotes: 0

Wouter van Nifterick
Wouter van Nifterick

Reputation: 24086

I see several problems in this code.

  • First of all, I think you want to do something like this:

    $myBib=new bib(); if($myBib->validate_bibid('612e436')) { ..do stuff.. }

    (or something similar)

    remember that __construct is not a normal method. It's a constructor, and it shouldn't return anything. It already implicitly returns a reference to the new instance that you've made.

  • Second, your validate_bibid returns either a boolean or an integer. You won't get immediate problems with that, but I personally don't like the style.

  • Third, you've declared a protected member $bibid, but you don't set or reference it anywhere. I'd expect it to be set in the constructor for example. After that you can just call validate_bibid without any argument.

This piece of code is obviously confusing you because it has some weird constructs and therefore doesn't behave in a normal way. I'd suggest to rethink and rewrite this piece from scratch.

Update:

Another issue:

I don't think this line does what you think it does:

 if(!$this->bibid = $this->validate_bibid($new_bibid)) {

You probably mean this:

 if(!$this->bibid == $this->validate_bibid($new_bibid)) {

 // Or even better:

 if($this->bibid <> $this->validate_bibid($new_bibid)) {

Upvotes: 2

Simon
Simon

Reputation: 37978

You can't return in a constructor in PHP - the object is still created as normal.

You could use a factory or something similar;

if(!$bib = Bib_Factory::loadValidBib('612e436')){
    echo 'Error Message';
} else {
    //do what I intend to do with the object
}

Or throw an exception in the constructor and use a try catch instead of the if statement.

Upvotes: 1

Related Questions