Not proper use of prepare statements in the sql repository example on documtentation page
Closed this issue · 4 comments
Hi there,
I have notice some kind of bug or performance issue:
In class Opulence\Orm\Repositories\Repository
we can see:
/**
* @inheritdoc
*/
public function add($entity)
{
$this->unitOfWork->scheduleForInsertion($entity);
}
and it seem to be valid.
But I found in your documentation located at https://www.opulencephp.com/docs/1.0/orm-repositories that you propose following code:
/** @var Post $post */
public function add($post)
{
$statement = $this->writeConnection->prepare(
'INSERT INTO posts (text, title, author) VALUES (:text, :title, :author)'
);
$statement->bindValues([
'text' => $post->getText(),
'title' => $post->getTitle(),
'author' => $post->getAuthor()
]);
$statement->execute();
}
which seems to be wrong.
Let's imagine that you will need to add 100 posts:
$dataMapper = new PostSqlDataMapper();
// Assume $unitOfWork is already instantiated
$repo = new Repository(Post::class, $dataMapper, $unitOfWork);
for ($i=0; $i < 100; $i++) {
$postToAdd = new Post($i, 'First Post', 'This is my first post');
$repo->add($postToAdd);
}
$unitOfWork->commit();
I haven't tried it yet, but from my point of view you will make new prepare statement for each post, instead of make it once and values as many as you want.
I feel this is something that should be considered at the developer's application level not the framework's.
If my app is a tiny stuff, the doc version would be fine. If i am building facebook, i should consider batching db queries.
If you're adding many posts in one go, I think the best approach would be to add another method to your data mapper, eg addMany(array $posts) : void
. add()
is reserved for single entity additions.
@adelowo It's quite common to have possibility to:
- create single object
- import many objects e.g. from CSV
But as @opulencephp wrote: we can use new method addMany(array $posts)