First attempt at OOP with PHP/MySQL

Question

I'm trying to get a feel for OOP with PHP/MySQL, so I attempted to write a program that will take a text input called "name" and store it in a database, then display the names that are stored. This is my first attempt at OOP so I'm not sure if I'm doing it right.

Any suggestions? Am I inserting the value properly? The table is called "names" and the column is "name."

Here are my two different files.This one is called template.php

<html>
<head>
</head>
<body>
<form action="template.php" method="post"> 
Person: <input name="person" type="text" /> 
<input type="submit" />
</form>
<table>
 <?php 

$insert_name = new MyController();
$insert_name-> getname($_POST['person']);


foreach ($names as $name); ?>
 <tr>
  <td><?php echo htmlspecialchars($name); ?></td>
<tr>
<?php endforeach; ?>
</table>
</body>
</html>

Now for my other file, index2.php

<?php

$connection = mysql_query("localhost","root","password") or die(mysql_error());
mysql_select_db("test",$connection) or die(mysql_error));

require_once("template.php");


class MyController
{
var $name;

function getname($new_name) { 
          $this->name = $new_name;      
    }


function insert(){
  mysql_query("INSERT INTO names(name) 
 VALUE ( "$this->name" )");       
}


function run()
{
$result = mysql_query("select * from names");
$names = array();
while ($row = mysql_fetch_array($result))
{
  $names[] = $row['name'];
}

include("template.php");
 }
 }

  $controller = new MyController();
  $controller->run();

?>

I stopped using class properties for storing values I will insert into my db, it got really tedious after a while. Also, I use prepared statements. — Frank, Dec 18 '11 at 20:05
The very first (code) line of your include is wrong (wrong function call). There is no error checking in your code after the (invalid) connection setup. Not using prepared statements... Make sure you follow the basics (including SQL injection) before you try and get fancy with objects. — Mat, Dec 18 '11 at 20:07
Good try, but you misunderstand the concept of OOP. Also, please take a look at PDO. — Madara's Ghost, Dec 18 '11 at 20:08
OOP isn't really applicable to your example code. At best, you would make your class represent a row from the database, with each column being a class property. But really, this example is just too small of a scale! — Julien, Dec 18 '11 at 20:28
In PHP 5 you really should set the visibility by public / protected / private — TimWolla, Dec 18 '11 at 20:36

score 1 · Accepted Answer · answered Dec 18 '11 at 20:45

1

You are generating your HTML all wrong. You should not be mixing complex PHP code (eg: mysql queries) with your HTML. Those two things should be in completely separate files, and most of the PHP part should be in it's own class. For example:

index2.php

<?php

require_once("dbinsert.php");

class MyController
{
  function run()
  {
    $insert_name = new datainsert();

    $insert_name->setname($_POST['person']);

    $result = mysql_query("select * from names");
    $names = array();
    while ($row = mysql_fetch_array($result))
    {
      $names[] = $row['name'];
    }

    include("my-template.php");
  }
}

$controller = new MyController();
$controller->run();

my-template.php

<html>
<head>
</head>
<body>

<form action="index2.php" method="post"> 
Person: <input name="person" type="text" /> 
<input type="submit" />
</form>
<table>
  <?php foreach ($names as $name); ?>
    <tr>
      <td><?php echo htmlspecialchars($name); ?></td>
    <tr>
  <?php endforeach; ?>
</table>

</body>
</html>

Alternatively, look into a proper templating language such as Smarty. I prefer it myself.

answered Dec 18 '11 at 20:45

Abhi Beckert

32,787
12
83
110

1

Sooo close to +1'ing you, then I saw the Smarty recommendation. hehe. Aside from that the answer is all good :) – Layke Dec 18 '11 at 21:01
Please do suggest anything you know of that's better than smarty. I didn't say I *like* smarty, only that I prefer it over all the other choices I know of. – Abhi Beckert Dec 18 '11 at 21:06
XSLT, XInclude and XLink are great for manipulating XML (including XHTML [and HTML]). – Saxoier Dec 18 '11 at 21:29
Thanks for the advice, but I'm still unable to get this to run. Why do you use require_once instead of include at the top of the file? Also, shouldn't it be require_once("my-template.php"); in that file you made? – user1104854 Dec 18 '11 at 21:34
a mixture of view models (think forms and grids) plus just straight included php files are what I use for templating. I was close to +1'ing you too, but the smarty recommendation tipped the scales against you. :/ – Tim G Dec 18 '11 at 21:40
Abhi, where exactly is the text box "person" referenced in the my-template.php file? Doe the require_once("my-template.php") allow you to get away with it? Shouldn't the line of code " $insert_name->setname($_POST['person']);" be in the my-template.php file? – user1104854 Dec 18 '11 at 21:46
@user1104854 This isn't the exact code I would use, it's just an overall example how to structure it. `require_once` and `include` do roughly the same thing, but `require_once` will take "file not found" errors much more seriously, and it makes sure the file isn't included twice. Almost all handling of the input field's value should not be in my-template.php, that should all be somewhere in the MyController class. But exactly how you structure it is up to you, I'm not suggesting you use this exact code, just pointing in the right direction. – Abhi Beckert Dec 18 '11 at 22:31
Thanks @Saxoier I will look into XSLT/etc, but I'm not entirely convinced on the idea of building a DOM structure to generate HTML. Seems a bit like using a bazooka to hammer in a nail? I definitely prefer smarty to actual php code, because `{$foo|escape}` is cleaner than ``, and the same for `{my_custom_tag foo=$bar}` vs ` $bar)); ?>`. Some of the php I write is generating seriously complicated HTML. Often ten thousand lines of php is hidden behind a few characters of smarty code. – Abhi Beckert Dec 18 '11 at 22:34
I updated my code in the original post I made, but I'm struggling to see how I can actually get it to display on the template.php page. Do I have to use pointers again somehow? – user1104854 Dec 18 '11 at 22:53
A simple suggestion that I have is not to use the `SELECT * FROM table` SQL syntax as it doesn't allow you to easily locate where certain columns are obtained - which is in turn potentially more difficult for debugging. See: http://stackoverflow.com/a/322216/102067 – ServAce85 Apr 18 '12 at 17:51
@ServAce85 I don't understand how `select *` would be more or less difficult to debug. I can understand optimisation arguments but not debugging ones. In my opinion, `select *` is usually plenty fast enough and I would only switch to something more specific after real world performance issues have shown their face. – Abhi Beckert Apr 23 '12 at 05:16
@AbhiBeckert As stated in the link I referenced: `2. You can more easily scan code where that column is being used.` This means that you can quickly and easily find where elements are obtained because they are explicitly listed. Hope this makes sense now. – ServAce85 May 01 '12 at 11:04

score 0 · Answer 2 · answered Dec 18 '11 at 20:32

on the second part of code snippet, opening tag is <?php not <?. Another thing would be to wrap your connection db query within try..catch block, so that it is easier to know when there's error. Better practice would be to use PDO for doing connection to the DB. Why? well, there's a a lot of articles about it already. One of them is here, I'd share with you http://net.tutsplus.com/tutorials/php/why-you-should-be-using-phps-pdo-for-database-access/

And also, the best practice is to sanitize input before inserting to the database. Sanitizing should be done on the method member that handles the data posted to avoid SQL injection; So i suggest you do:

function setname($sent_name){
            $sent_name = mysql_real_escape_string($sent_name);
            $this-> insert_name = $sent_name ;
}

When creating class that is invoked as a new object (if not working with just plainly static variable), you probably want to create a constructor function where the initial state of the private variables are created. The regular convention is also to use Uppercase for the class name. So, in your class, you may want to do this instead:

class DataInsert{

var $insert_name;

function __construct(){
//initialize
}

function setname($sent_name){
            $sent_name = mysql_real_escape_string($sent_name);
            $this-> insert_name = $sent_name ;
}

function dbinsert(){
    mysql_query("INSERT INTO names(name) 
    VALUE ( "$this->insert_name" )");       
    }
}

Hope that helps. In the end, have fun with PHP. The next thing is to learn the MVC part then (if you havent been exposed to such design pattern) where there are a few frameworks available for PHP; i.e .cake, zend.

I myself have not done much PHP for a while now since I'm now mainly focusing on ruby on rails and node.js. I think rails in particular is much more fun tow work with. So, another suggestion for you is to take a look at them in the future (again, if you haven't known them yet). thanks.

First attempt at OOP with PHP/MySQL

2 Answers2