coen-hyde / Shanty-Mongo

Shanty Mongo is a mongodb library for the Zend Framework. Its intention is to make working with mongodb documents as natural and as simple as possible. In particular allowing embedded documents to also have custom document classes.
Other
200 stars 52 forks source link

Isset vs array_key_exists #80

Closed gwagner closed 12 years ago

gwagner commented 12 years ago

It has been proven time and time again that isset is much faster than array_key_exists in virtually all tests, yet the code is littered with array_key_exists. the only difference between array_key_exists and isset, isset will return false if the var is set and equals null unlike array_key_exists.

Now my question is, is the community of users really attached to the logic of nulls (which IMHO are very poor practice to use in PHP) or is this something that can be converted to a true / false scenario with out a third layer of null mucking up strict logic.

coen-hyde commented 12 years ago

I'm not attached to array_key_exits. I was under the impression that the speed difference was pretty much insignificant. But if you fork current master and replace the array_key_exists with issets and the tests still pass then I'd be very happy to merge the pull request.

I know you've already made these modifications in your branch but it's close to impossible to merge your pull request as it has so many different features or modifications. It's best to keep the pull requests to single responsibility.

coen-hyde commented 12 years ago

I'm going to continue to ignore massive pull requests. Because this is a database library and we play with peoples data. I can't merge things willy nilly. A lot of the modifications you have made are quite valid, but because they are mixed in with other modifications they have to be assessed as a whole and even if I believe I could assess all the changes it's just too much risk.

I've actually appointed @tholder as the new maintainer as I don't actively use Shanty Mongo anymore (I'm doing development in other languages). You're more than welcome to continue to maintain your own branch, that's how the open source world works. But if you'd like your changes in the main branch (and I'd like them in here too) you're going to have to follow these rules:

  1. Follow the current coding style
  2. Write tests for new functionality, and make sure existing tests pass
  3. Keep pull requests to single responsibility and small

I would suggest branching your fork from before any of your modifications, merge in this current master, then create a separate branch for each feature/modification. That way you can create pull requests don't depend on any of your existing modifications. That is the way most people contribute of other projects.