0

I'm trying to switch from procedural to OOP php, I can connect to a database and use it fine in procedural however after a few hours of head scratching, google searches and little tweaks/changes I concede defeat and conclude I must be missing something.

I have two files, my IDE marks that the syntax issue is apparently with the "function"'s part of the first file (test.php) but I just can't figure out what, does any body have any ideas? the two files are below. Sorry for the formatting but for some reason I can't tab within this textarea.

I'm assuming its something to do with a lack of constructor. I've tried to add one in instead of the config function but I still have syntax issues. In documentation/tutorials I can find mention of constructors in the php and functions in the html but nothing seems to explain how they work together with an example.

Disclaimer: both files are surrounded by <?php ?> tags and I am not addressing security measures currently. Walk before you can run and all that. I've also tried to throw a doctype at the beginning of test.php and pad it with <html> <head> and <body> tags where appropriate but to no avail. Using Apache 2.2.22 and PHP 5.3.13.

// ------- Test.php start -------
include 'obj_lib.php';

$db = new database;
$db = function config('test', 'root', '', 'localhost');
$db = function connect();
$db = function test_database();
// ------- Test.php end -------

// ------- obj_lib.php start -------
class database
{
  private $username;
  private $password;
  private $database;
  private $server;
  private $db_handle;
  private $db_found;

   public function config($indatabase=null, $inusername=null, $inpassword=null, $inserver=null)
   {
   $this->username = $inusername;
   $this->password = $inpassword;
   $this->database = $indatabase;
   $this->server   = $inserver;
   }

   public function connect()
   {
   $this->db_handle = mysql_connect($this->server, $this->username, $this->password);
   $this->db_found = mysql_select_db($this->database, $this->db_handle);
   }

   public function test_database()
   {
   echo "$this->username"."$this->password"."$this->database"."$this->server"."$this->db_handle"."$this->db_found";
   }
 }
// ------- obj_lib.php end -------
PeeHaa
  • 71,436
  • 58
  • 190
  • 262
Dicckk
  • 61
  • 6
  • Your username makes me hesitant to answer – Osa Nov 19 '12 at 23:46
  • 2
    Other than what xelber pointed out in his answer, you're doing a couple of things wrong: you should have all that good stuff in the constructor, since you'll presumably call `config()` and `connect()` every time you load create a new instance of `database`. Somewhat as a direct result of the previous point, you probably shouldn't need the `$username`, `$password`, `$server` and `$database` member variables since you'll only ever use them once (in construction). – NullUserException Nov 19 '12 at 23:49
  • In addition to the above, I would strongly advise you to switch to mysqli instead of mysql. That will be removed from the api in the near future. – Digitalis Nov 19 '12 at 23:51
  • 3
    There is a small flaw in your analogy "Walk before you can run" - addressing security wouldn't be running, it would be making sure you know how to stand upright. In other words, it makes sense to think about security-related issues before starting to code. On the other hand, it is, of course, not wrong to freely experiment with a new programming technique. Just make sure you do it in a safe environment and everything's fine. – MvanGeest Nov 19 '12 at 23:51
  • Should you feel lost in the oop syntax, try this. It is an excellent and very simple tutorial to follow and can also serve as a cheatsheet: http://www.codewalkers.com/c/a/Programming-Basics/Beginning-Object-Oriented-Programming-in-PHP/1/ – Digitalis Nov 19 '12 at 23:54
  • @Dick I would probably rewrite the class to something like this: http://pastebin.com/igi7QtkG – NullUserException Nov 19 '12 at 23:57
  • @NullUserException But again, switch to mysqli. It saves a lot of future work. – Digitalis Nov 19 '12 at 23:59
  • @Digitalis Yes. At this point I'd probably use PDO but I wasn't trying to point out which extensions the OP should use, just that from an OOP perspective some of the code isn't making much sense. – NullUserException Nov 20 '12 at 00:03
  • @NullUserException Nice one, though PDO is a lot harder to grasp at this point of the journey, I reckon :) – Digitalis Nov 20 '12 at 00:07
  • @Digitalis I disagree, specially when it comes to parameterized queries - they are a lot cleaner in PDO. – NullUserException Nov 20 '12 at 00:09
  • @NullUserException It is cleaner, however, I personally found it quite a big step to take. PDO has some weird new things compared to the old way of handling the database. I agree completely that its more secure and a lot cleaner. – Digitalis Nov 20 '12 at 00:11

1 Answers1

3
$db = new database;
$db = function config('test', 'root', '', 'localhost');
$db = function connect();
$db = function test_database();

Should be

$db = new database;
$db->config('test', 'root', '', 'localhost');
$db->connect();
$db->test_database();
xelber
  • 4,197
  • 3
  • 25
  • 33
  • Plus one, just because you typed quicker :) – Digitalis Nov 19 '12 at 23:46
  • @Digitalis I've rejected your edit because the parens are not required; it's just a matter of stylistical preference. `$db = new database;` and `$db = new database()` are functionally equivalent. – NullUserException Nov 19 '12 at 23:55
  • @NullUserException Really? Is that a settable option because it throws an error here, strict standards. Thanks btw, good catch! – Digitalis Nov 19 '12 at 23:57
  • 1
    @Digitalis See: http://stackoverflow.com/a/3873144/396458; although as a someone used to Java/C#/C++ syntax I'd prefer using the parentheses – NullUserException Nov 20 '12 at 00:00
  • Yes, that's why I made the edit. It actually looks quite confusing. – Digitalis Nov 20 '12 at 00:08
  • parentheses are really only needed if you are passing params to the class like for dependency injection. – Lawrence Cherone Nov 20 '12 at 00:09
  • I've ticked @Xelber because it worked thanks a lot! Digitalis, mysqli might be a good plan, I'll take a look at the link too, thanks! Mvan, I put that in because every answer I've seen that isn't an end-user, ultra-secure fort-noxesque bit of code gets around 9001 responses which completely avoid that actual question and end up sounding like a pit of screaming children going "OMG THATS NOT SAFEEEE!!!!1". – Dicckk Nov 20 '12 at 01:22