Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link, and the comments, at the bottom of this answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => $productID,'Status' => 'Stock'));
$shipped = $query->execute(array('ProductID' => $productID,'Status' => 'Shipped'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: httphttps://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link, and the comments, at the bottom of this answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => $productID,'Status' => 'Stock'));
$shipped = $query->execute(array('ProductID' => $productID,'Status' => 'Shipped'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link, and the comments, at the bottom of this answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => $productID,'Status' => 'Stock'));
$shipped = $query->execute(array('ProductID' => $productID,'Status' => 'Shipped'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: https://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394
added 28 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link, and the comments, at the bottom of mythis answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => 123$productID,'Status' => 'Stock'));
$shipped = $query->execute(array('ProductID' => $productID,'Status' => 'Shipped'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link at the bottom of my answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => 123,'Status' => 'Stock'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link, and the comments, at the bottom of this answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => $productID,'Status' => 'Stock'));
$shipped = $query->execute(array('ProductID' => $productID,'Status' => 'Shipped'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394
added 2 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link at the bottom of my answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => 123,'Status' => 'Stock'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classclasses, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link at the bottom of my answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => 123,'Status' => 'Stock'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO class, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394

You've made a nice and small DAO (Data Access Object). Small is good.

I don't have a complete answer to all your questions, but I have some remarks:

A: Extend, don't obscure

You've made the PDO object private in your class. This means you cannot access any of the functionality of the PDO object outside your class. Even running a simple direct query is not possible: `PDO::query($statement)`. Why do this? It's better to extend the PDO class like this:
class MyPDO extends PDO
{
}

Then everything will still be accessible and you simply add to it. I would only add those things that clearly belong to the PDO class, like making a connection and transactions. A class should have only one job to do. (And also read the link at the bottom of my answer, why you should be careful adding things)

B: Lean and reusable classes

There would be an obvious way to split your class, in almost the same way as it was done for the PDO class: Database related and query related. You could write a seperate class to do queries. This allows you to keep a query and run it several times, binding other values to it. You would have to seperate creating the query, and executing it, into at least two methods. In my own DAO I have build a basic query class and extended that to have the CRUD functionality (Create, Read, Update and Delete):
class query
{
}
class selectQuery extends query
{
}
class insertQuery extends query
{
}
class updateQuery extends query
{
}
class deleteQuery extends query
{
}

This way I can put all common functionality in the query class. Doing a query can now looks like this:

$query = new selectQuery($handleDB,'items_table','ProductID, COUNT(*)');
$query->where("Trashed = 'false'")
 ->where('ProductID = :ProductID')
 ->where('Status = :Status')
 ->groupBy('ProductID');
$stock = $query->execute(array('ProductID' => 123,'Status' => 'Stock'));

Note that I can add methods like where() and groupBy() by chaining them. Many arguments can be strings or arrays. A string like 'ProductID, COUNT(*)' is converted to an array, but could also have been: array('ProductID','COUNT(*)') or ['ProductID','COUNT(*)'] when your PHP can handle it.

C: Why do it?

You should only create a DAO class if you can add something to the existing PDO classes, not to simply wrap data access in your own class. That would be pointless. Does your class help you to prevent SQL bugs? Does it make constructing correct SQL statements easier? Does it do profiling? Does it provide shorter and better to read code? Add an extra layer of data protection? In other words: Why is your class better than what's already there? Your class is no good, if you cannot answer that question. Read and shiver: http://codereview.stackexchange.com/questions/29362/very-simple-php-pdo-class/29394#29394
added 95 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24
Loading
added 150 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24
Loading
added 150 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24
Loading
added 12 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24
Loading
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24
Loading
lang-php

AltStyle によって変換されたページ (->オリジナル) /