Page 1 of 1

Proper use of classes, to borrow variables from others?

Posted: Fri Aug 19, 2005 8:10 am
by Black Unicorn
Hi all.

I have a situation where one of my classes are getting way too big. It's used for a forum and its methods are responsible for writing various parts of it, i.e.

Code: Select all

$_SESSION["board"]->WriteHeader()
$_SESSION["board"]->WriteNavigation()
etc.

I am now working on a messaging part, and contemplating making a class for it specifically, so I'd get something like:

Code: Select all

$msg = new Message();
$msg->senderID = $senderID;
$msg->message = $message;
$msg->Dispatch();
However, for the message class, would it be sensible to "borrow" MySQL handles and other stuff from other objects rather than loading it into the object separately?
If I have something like:

Code: Select all

class Message{
 var $mysql;
 var $conn;
 function Message($mysql,$conn,$other){
   $this->mysql = $mysql;
   $this->conn = mysql_connect ...
 }
}
or would it be OK to do something like ...

Code: Select all

class Message{
 var $privatevar1;
 var $privatevar2;
 function Message(&$msg){
   $this->message= $msg;
   $this->messageID = $_SESSION["board"]->Query("SELECT `id` FROM `table`");
   $_SESSION["board"]->Query("INSERT INTO `msg` (`message`) VALUES ('$msg' WHERE `id`=".$this->userID.")");
 }
}
I hope these examples make my dilemma clear. On one hand, I'd like not to put too much in one object, such as making the message system part of the main board object, but on the other hand I'd like to keep various variables within the same object, which means passing a lot of information as parameters.

I think I made a mistake putting the mysql stuff into the main board object and not standalone, but sofar it hasn't been a problem.
Any suggestions are welcome.

Thanx,
H
}

Posted: Fri Aug 19, 2005 8:20 am
by feyd
you should have a Database object, actually. This object reference would then potentially be passed around, however I'd opt to keep the reference in an application class that acts as a gateway to the Database object.

Posted: Fri Aug 19, 2005 8:49 am
by Black Unicorn
Thank you for the insight.

By using an application class to store the reference, would it be something like:

Code: Select all

$board = new Forum();
$app = new Application();
where application starts with something like:

Code: Select all

class Application{
  var $db;
  var $a;
  var $b;
  function Application(){
    $this->db = new Database();
  }
  function Query($q){
    return $this->db->Query($q);
  }
}
I am new to OOP but I already have benefited a lot from its logic. My biggest problem is having too many different ways to achieve the same thing.
However, I am having trouble seeing why calling an object's method by proxy has any advantages.

Sincerely,
H

Posted: Fri Aug 19, 2005 8:57 am
by feyd
it's mostly because you shouldn't really internalize methods that are outside the object's work zone. You don't have to "proxy" all (or any of) the methods from the database class in the application class. Using a database class alone helps a lot, because you could at some point change the DBMS you use, or change methodologies. I prefer making a small amount of changes when I change the idea of how I want something to work. So the very minimum would be a Database class.. I'm sure McGruff can demonstrate far more flexing of this information and explaination than I can :)

Posted: Fri Aug 19, 2005 10:51 am
by patrikG
Inheritance is key. Take your parent-class

Code: Select all

//include the file of the child-class here
class Application{
  var $db;
  var $a;
  var $b;
  function Application(){
    $this->db = new Database();
  }
  function user($id){
     $this->user["id"]=$id;
     $this->user["message"]=Message::getMessage();
  }
}
child-class

Code: Select all

class Message extends Application{
 function getMessage(){
   $this->db->Query("SELECT foo FROM bar WHERE userID='".$this->user["id"]."'");
   ...
   return $result;
 }
}
$this->user["message"] now contains the messages stored in the db with a particular user-id. You do not need to instantiate the child-class. PHP's scope resolution operator is key here: "::" as in

Code: Select all

$this->user["message"]=Message::getMessage();
The child-class can obviously access it's parents methods and properties (such as $this->db).

Posted: Fri Aug 19, 2005 10:53 am
by Black Unicorn
Thanks, I see where you're coming from and have to agree.

I think I'll start making a standalone DB class and do the same with various others.
At least I am good enough at OOP to intuitively feel I'm making poor design choices.

This stuff is much fun at times :)

best wishes,
H

Posted: Fri Aug 19, 2005 11:02 am
by nielsene
Hmm I think patrikG's suggestion is abusing inheritence.

A message isn't a subclass of an Application. If the only reason you're subclassing it is to get access to a DB object, then you should probably re-think that.

I'd normally go with a Registry in this kind of situation. The refernce to the DB is needed so many places... its not clean to thread it through every class that needs it. So start with a Registry (so you can do

Code: Select all

$db=Registry::getDB();
when you need it. Then slowly tease all the databasse references out-toward gateway/mapper classes so that the innards of the system don't know about the DB. Finally heopfully you can get rid of the Registry and stick only with the gateways/mappers which are given the DB connection explicitly as that's their job.

Posted: Fri Aug 19, 2005 11:04 am
by patrikG
nielsene wrote:Hmm I think patrikG's suggestion is abusing inheritence.

A message isn't a subclass of an Application. If the only reason you're subclassing it is to get access to a DB object, then you should probably re-think that.

I'd normally go with a Registry in this kind of situation. The refernce to the DB is needed so many places... its not clean to thread it through every class that needs it. So start with a Registry (so you can do

Code: Select all

$db=Registry::getDB();
when you need it. Then slowly tease all the databasse references out-toward gateway/mapper classes so that the innards of the system don't know about the DB. Finally heopfully you can get rid of the Registry and stick only with the gateways/mappers which are given the DB connection explicitly as that's their job.
True. My point was to introduce inheritance, not design patterns nor application design.

Posted: Fri Aug 19, 2005 11:13 am
by nielsene
patrikG wrote:
nielsene wrote:Hmm I think patrikG's suggestion is abusing inheritence.

A message isn't a subclass of an Application. If the only reason you're subclassing it is to get access to a DB object, then you should probably re-think that.

I'd normally go with a Registry in this kind of situation. The refernce to the DB is needed so many places... its not clean to thread it through every class that needs it. So start with a Registry (so you can do

Code: Select all

$db=Registry::getDB();
when you need it. Then slowly tease all the databasse references out-toward gateway/mapper classes so that the innards of the system don't know about the DB. Finally heopfully you can get rid of the Registry and stick only with the gateways/mappers which are given the DB connection explicitly as that's their job.
True. My point was to introduce inheritance, not design patterns nor application design.
Well, too many people abuse inheritence, often because of simplistic tutorials, etc.

The "simple" answer here, I think would be

Code: Select all

class Message {
  var $db;
  var $msg;
  functon Message(&$msg,&$db) {
   $this->db=$db;
   $this->msg=$msg;
   $this->db->query(..);
}
So yes, I'ld explicitly pass in an already created DB handle. If the class needs to use it outside the constructor, save it to the class variablee. Now this assumes that you'll always create a message in a context that has an available DB reference. This will in turn drive the progression towards a registry anyways...

I don't think the OP's idea of sticking the DB into the session and pulling it from there is a good idea. Mainly because DB connections don't survive the page cycle and therefore its likely to cause confusion -- you're using the SESSION as purely an alias for GLOBALS in that case and that's not goof.

Posted: Fri Aug 19, 2005 11:54 am
by timvw
Put simply:

- A Message "has a" link to a Database Instance. As soon as you see the relation "has" you should think about composition.

- A Message "is a" link to a Database Instance. As soon as you see the relation "is" you should think about inheritance.


As nielsene already said, it can become a major pita if you create an instance of the Database Instance in the Message because as soon as you write a SuperDBI, you will have to change all the constructor calls and replace them with a new SuperDBI(); call instead. To avoid this situation there are a couple of solutions. Although the code is in Java, this article tries to explain some of them. If i understand it right, the example that nielsene gave would be classified as a ServiceLocator in the article.

Posted: Fri Aug 19, 2005 11:57 am
by nielsene
Yes my first choice would be a Registry aka a ServiceLocator. Second choice is composition via explicit parameter passing.

Re: Proper use of classes, to borrow variables from others?

Posted: Fri Aug 19, 2005 12:39 pm
by McGruff
Black Unicorn wrote:However, for the message class, would it be sensible to "borrow" MySQL handles and other stuff from other objects rather than loading it into the object separately?
As has been mentioned, it's better to have a separate database object. The key is to identify different responsibilities: sending a message and encapsulating a database connection are two different things.

Want to try creating a message class with test driven design? The idea is you write a little test, then write a little code which passes the test. Repeat until done.