1

I am trying to use OOP PHP to access my mongodb collection. Yet on the page it returns nothing, but when I look in my apache error log it states

PHP Notice: Trying to get property of non-object in main.php on line 22

Here is the code I'm using: db.php

class db {
    static $db = NULL;

    static function getMongoCon()
    {
        if (self::$db === null)
        {
            try {
                $m = new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}');

            } catch (MongoConnectionException $e) {
                die('Failed to connect to MongoDB '.$e->getMessage());
            }
            self::$db = $m;
        }
            else
        {
            return self::$db;
        }
    }
   }

main.php

//load db files
require_once('db.php');

class test{
   public function __construct() {
       echo $this->output();
   } 
   public function output() {
       $con=db::getMongoCon();
       $db=$con->database;
       $test=$db->collection;
       $n=$test->find();
       print_r($n);
   } 
}

new test();

This all works using procedural code, and I have been able to insert data in this method as well - so it should work (I have removed the database details here for obvious security reasons).

Note: I have read this but it is still not working.

Community
  • 1
  • 1
Daniel
  • 72
  • 1
  • 10
  • Did it give any clue as to what line the error was on? you are accessng quite a few properties there, short of dishing out every possible problem one by one I am not sure how we can help without knowing what line caused the error. – Sammaye Apr 25 '13 at 14:44
  • Sorry, I never thought about that - edited to add. – Daniel Apr 25 '13 at 14:47
  • @Daniel: just read my answer: drop the `else` in your getMongoCon method, I bet you line 22 is (near to) `$db=$con->database;` but as I explained, if `db::$db` is null, the `getMongoCon` returns null, and you're effectively doing `$db = null->database;` === accessing a property of null, hence: property of a non-object – Elias Van Ootegem Apr 25 '13 at 14:51

1 Answers1

2

It's a very simple mistake. You're using the factory pattern, but the first time you're calling the getMongoCon method, it'll return null:

class Db
{//convention: Upper-Case classes
    private static $_db = null;//using this pattern a public static is a bad idea
    public static function getMongoCon()
    {//^^ best specify public/private/protected
        if (self::$db === null)
        {//first time called: this is true
            try {
                //I'd assign immediatly here, but hey...
                self::$db = new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}');
                $m = new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}');
            }
            catch (MongoConnectionException $e)
            {
                die('Failed to connect to MongoDB '.$e->getMessage());
            }
            self::$db = $m;
        }
        //!!leave out else to fix!!
        return self::$db;
    }
}

As you can see, all I did was leave out the else, no matter what the value of self::$db was you have to return it. but if it was null, you'll never get in the else branch, that contained the return statement.
There is also no real reason for that throw-catch block. You clearly rely on that DB-connection for your code to work, if it doesn't just, there is no backup AFAIK, so just let it throw (and halt).

Just for completeness, here's how I'd write the above code:

class Db
{
    private static $_db = null;
    public static function getMongoCon($new = false)
    {//allow for another connection, you never know...
        if (self::$_db === null)
        {
            self::$_db = new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}');
        }
        if (true === !!$new)
        {
            return new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}');
        }
        return self::$_db;
        //or, short 'n messy:
        return (!$new ? self::$_db : new Mongo('mongodb://{user}:{password}@{host}:{port}/{database}'));
    }
}
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149