Get 1 row from a database table

[simonh]

Hello!

I have been working with PHP for a while now and I just started to learn myself OOP. I would like to get some critique on my class that gets 1 row from a database table.

CRUD.php:

class ReadWhere {
	public $errorMessage;

function setErrorMessage($message) {
	$this->errorMessage = $message;
}

function findRow($sql) {
	$query = mysql_query($sql);
	$rows = mysql_num_rows($query);
	if($rows > 0) {
		$data = mysql_fetch_assoc($query);
		foreach ($data as $field => $value) {
			$this->{$field} = htmlentities($value);
		}
		$this->errorMessage = false;
	} else {
		$this->noRows = false;
	}
}
}

index.php:

if(!empty($_GET['id'])) {
	if(is_numeric($_GET['id'])) {
		$id = $_GET['id'];
	} else {
		echo "Eh...";
		exit;
	}
} else {
	echo "Eh...";
	exit;
}

require "includes/connect.php";
require "classes/crud.php";

$read = new ReadWhere;
$read->setErrorMessage("The profile you were looking for was not found");
$read->findRow("SELECT * FROM users WHERE id = '".mysql_real_escape_string($id)."' LIMIT 1");

echo $read->errorMessage;

if(!isset($read->noRows)) {
echo $read->name;
echo "<br />";
echo $read->from;
echo "<br />";
echo $read->email;
}

Thanks in advance!

[Weedpacket]

I wouldn't put the htmlentities() call in the method, because I might want the row for something else.

I'd probably also use [man]PDO[/man] instead (or, rather, wrap your classes around PDO so that you can specify your own interface), since, for example, the job of fetching one row from a result set and creating from it an object with fields named for the columns is:

$object = $result_set->fetchObject();

. Passing an additional class name allows the returned object to be an instance of that class.

[gammaster]

Hello!

1. I dont like the name og your class. I know classes is named in all ways this days, but in general classnames should be nouns. A "RowFetcher" or something would be a better name in object oriented terms.

2. What Weedpacket say. The htmlentities call shouldnt be in there if you want to write reusable and independent code. Next time you want to reuse this class you maybe dont want it to do this.

3. An error message has nothing to do in a class like this. It should return the result array if found, an empty array if not found and maybe false if you have some query error checking in there. It should be the user of the class that defines the error messages when it get the result from the method.

Sorry for my bad english.