colinmollenhour / mongodb-php-odm

A simple but powerful set of wrappers for using MongoDb in PHP (also a Kohana 3 module)
BSD 3-Clause "New" or "Revised" License
208 stars 50 forks source link

update_safe accepts $upsert and $multi options #33

Closed panrafal closed 11 years ago

colinmollenhour commented 11 years ago

Actually, looking back at this I don't get the motivation for this patch.. update_safe already allows for overriding these options:

$collection->update_safe($criteria, $update, array('multiple' => TRUE, 'upsert' => TRUE));

So now there are two ways to override multiple and upsert so I think this should be reverted..

sergeyklay commented 11 years ago

yep, but presently update_safe is:

public function update_safe ($criteria, $update, $options = array(), $upsert = false, $multi = false)
{
   ...
}

in my opinion $upsert and $multi not needed here.

Actually, if you look at the php-prototype of update method, we can see the following: public bool|array MongoCollection::update ( array $criteria , array $new_object [, array $options = array() ] ).

In his turn MongoDB defines this method as follows: db.collection.update(query, update[, options])

By this, I don't see the point of invent additional arguments for update_safe.

See please:

Also for a better understanding, I suggest to rename $update to $new_object

panrafal commented 11 years ago

From my pov it's more convenient to have the most used parameters available right on the function - especially if you use any kind of code completion. But I understand yours, especially the part with original mongo api :)

colinmollenhour commented 11 years ago

@panrafal If it weren't for the redundancy which causes ambiguity then I'd agree with you. Reverted.