On Mon, 20 Dec 2010 19:41:29 -0000, Matthew Turland <tobias...@gmail.com>
wrote:
Thanks to comments from Gustavo Lopes, I've removed the removeCommon
method from my patch. I honestly wish I could say why I didn't realize
his point before I submitted the patch in the first place, but I
appreciate the feedback. I've attached the amended patch files, which
include only the removeUncommon method, which I definitely know does
not already exist in the class.
php-src-5.3-patch.diff is against php/php-src/branches/PHP_5_3
phpdoc-en-trunk-patch.diff is against phpdoc/en/trunk
As for the comments regarding the naming conventions, I do agree to a
certain extent. However, I would like to remain consistent with the
format of names of existing methods. I suggest a separate patch be
submitted with method aliases to deal with that situation. If anyone
has a better name for the removeUncommon method, I'm open to
suggestions.
Any further feedback and/or approval/merging would be appreciated.
"Remain[ing] consistent with the format of names of existing methods"
would imply calling the method "retainAll" since the set operations
"addAll" and "removeAll" are presumably inspired in Java and "retainAll"
is the name given to the method that has these semantics. I would be
relatively comfortable with something like "removeExceptAll" or
"intersectWith" but the first is awkward and the second would be more
appropriate for a method that would return a new set instead of relying on
collateral effects.
I think "removeUncommon" is an infelicitous choice, it forces you to think
about what the "uncommon" elements in the two sets are, which is
unnecessary to understand the semantics of the method; all you have to do
is remove everything from the first that isn't in the second (or retain
the common ones), i.e., the uncommon ones on the second don't matter at
all. "retainAll" and "removeExceptAll" express this clearly,
"intersectWith" has an immediate visual appeal, "removeUncommon" is a big
cognitive burden.
Other than that, +1
--
Gustavo Lopes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
