Cleaner static variable use

PHP
1 

$db = self::$var1;
$coll = self::$var2;
$rDb = self::$var3;
        
$m->$db->$coll->find();

As you can see I’m using the variables in the object $m; I first have to re-assign the static variables, as the self::$var can’t be used in an object. Any ideas on a cleaner way to get around this?

2

Hmmm… I haven’t tested this but I thought the following was possible:

$m->{self::$db}->{self::$coll}->find();

Granted, personally, I’d take this as a warning that your use of static may be ill-advised in this scenario and consider why you made them static to begin with.

3

Well, they are for use within a few functions, but within only this single class, and never to be altered by a child or outside source (EDIT: or even the class itself, they just need defined once and never altered), it just seemed logical to me, minus the fact that I’m redecalring them every time I want to use them in an object such as this.

This is for use in a MongoDB class, so I’m declaring the db and collection to be used across the class one time, but using it many.

EDIT: This does work, much cleaner. Thank you.

4

An altered example of what I’m doing:


class Album {
    private static $AlbumDb = 'my_db';
    private static $AlbumColl = 'my_coll';
    
    public $mongo;
    
    public function construct() {
        $this->mongo = new MongoClient();
    }
    
    public function AddAlbum($AlbumName) {
        try {
            $this->mongo->{self::$AlbumDb}->{self::AlbumColl}->insert(array('album_name' => $AlbumName));
        } catch(MongoCursorException $e) {
            Throw New Exception($e);
        }
    }
    
    public function DeleteAlbum($AlbumName) {
        try {
            $this->mongo->{self::$AlbumDb}->{self::AlbumColl}->remove(array('album_name' => $AlbumName));
        } catch(MongoCursorException $e) {
            Throw New Exception($e);
        }
    }
}
5

Why not just set the db once in the constructor and use that throughout?


class MyDB
{
  private $db;

  public function __construct($db, $collection)
  {
    $this->db = new MongoCollection();
    $this->db->selectDb($db);
    $this->db->selectCollection($collection);
  }

  public function doSomething()
  {
    // use $this->db to query
  }
}
6

You know this is one of those moments you want to slap yourself, just a little, as I used to write it like this.

Though I would only separate the mongo collection and the connection as the original connection does get re-used for other collections. Thanks for the quick reality check

7

Also, I would create a sort of superclass for basic functionality so you’re repeating yourself over and over again with different names and parameters.


abstract class Model
{
  private $dbName;
  private $collectionName;

  public function __construct($dbName, $collectionName)
  {
      $this->db = new MongoClient();
      $this->db->selectDb($dbName);
      $this->db->selectCollection($collectionName);
  }

  public function insert($data)
  {
    try {
      $this->db->insert($data);
    } catch(MongoCursorException $e) {
       throw new Exception($e); // why?
    }
  }

  public function delete($data)
  {
    try {
      $this->db->delete($data);
    } catch(MongoCursorException $e) {
       throw new Exception($e); // why?
    }
  }
}

class Album extends Model
{
  public function __construct()
  {
    parent::construct('my_db', 'my_collection');
  }

  // if you want you can add functions like
  // this to hide implementation details
  // like the 'album_name' key. Or you
  // can just call ->insert and ->delete
  // from "outside"
  public function addAlbum($albumName)
  {
    $this->insert(array('album_name' => $albumName));
  }

  public function deleteAlbum($albumName)
  {
    $this->delete(array('album_name' => $albumName));
  }
}

That way you’ve also moved all the try {} catch {} stuff out of the way to the superclass and you can concentrate on real business logic in the implementing classes.

8

Well perhaps I dumbed my example down too far, and too much into a web format. Most of my functions are very complex, usually 100-200 lines of code, and creating a generic insert, delete, etc doesn’t actually contribute anything unless I plan to write in retries.

9

Just wanted to point out, this is what I meant by “this may be a warning of an ill-advised scenario and to question why you are doing it”, I just didn’t have the time to think/write out a better approach (plus I didn’t have your code at hand :)) Thank you @rpkamp ; for having the time to do that.

I believe the approach you wrote will be far easier to manage. I also like the idea of pushing generic insert/delete/update syntax to the super-class, let it worry about how the database should be interacted with, and allow your other classes to just push data to it. The less data access you can do within the business layer the better :slight_smile: And when you find yourself in a spot where you need to do more, extend your super-class to handle that unique scenario and call it from your business layer.

10

Actually I believe the original reason was that my shop is trying to adopt some standards, and I believe we were trying to get our config files up to the top of classes for quick editing. I suppose the constructor is close enough.

Thinking on this further, this might not be a bad idea to create a superclass. I’m usually running very large batch files, and having some generic error handling such as a retry that I’m usually writing out anyway would be beneficial.