FAQ
Attached are source/test and manual patches for the addition of two
methods to the SplObjectStorage class. These methods, removeCommon and
removeUncommon, add support for the difference and intersection set
operations, respectively.

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

Any feedback and/or approval/merging would be appreciated.

Regards,

Matthew Turland

Search Discussions

  • Gustavo Lopes at Dec 19, 2010 at 2:50 pm

    On Sun, 19 Dec 2010 05:33:56 -0000, Matthew Turland wrote:

    Attached are source/test and manual patches for the addition of two
    methods to the SplObjectStorage class. These methods, removeCommon and
    removeUncommon, add support for the difference and intersection set
    operations, respectively.
    I'm sorry, I must be missing something because what exactly is the
    difference between removeCommon and removeAll?

    I think it's unfortunate that SplObjectStorage chose the Java naming of
    the set operation methods instead of following PHP's choice for arrays:

    * "diff" for the complement
    * "intersect" for the intersection
    * "merge" for the union

    (You could argue that Java's naming is more appropriate because, contrary
    to the array functions, the methods have collateral affects that are
    better expressed with "remove" and "add".)

    In any case, consistency would dictate the difference should be called
    "removeAll", intersection should be called "retainAll" and the union
    should be called "addAll".

    --
    Gustavo Lopes
  • Hannes Magnússon at Dec 19, 2010 at 3:03 pm

    On Sun, Dec 19, 2010 at 15:49, Gustavo Lopes wrote:
    On Sun, 19 Dec 2010 05:33:56 -0000, Matthew Turland wrote:

    Attached are source/test and manual patches for the addition of two
    methods to the SplObjectStorage class. These methods, removeCommon and
    removeUncommon, add support for the difference and intersection set
    operations, respectively.
    I'm sorry, I must be missing something because what exactly is the
    difference between removeCommon and removeAll?
    Is this just bikeshedding over the method names, or did you not bother
    reading the patches? (the doc patch even includes examples).

    -Hannes
  • Gustavo Lopes at Dec 19, 2010 at 3:16 pm

    On Sun, 19 Dec 2010 15:12:17 -0000, Hannes Magnússon wrote:
    On Sun, Dec 19, 2010 at 16:05, Gustavo Lopes wrote:
    On Sun, 19 Dec 2010 15:03:22 -0000, Hannes Magnússon
    wrote:
    On Sun, Dec 19, 2010 at 15:49, Gustavo Lopes <glopes@nebm.ist.utl.pt>
    wrote:
    On Sun, 19 Dec 2010 05:33:56 -0000, Matthew Turland
    wrote:
    Attached are source/test and manual patches for the addition of two
    methods to the SplObjectStorage class. These methods, removeCommon
    and removeUncommon, add support for the difference and intersection
    set
    operations, respectively.
    I'm sorry, I must be missing something because what exactly is the
    difference between removeCommon and removeAll?
    Is this just bikeshedding over the method names, or did you not bother
    reading the patches? (the doc patch even includes examples).
    I did, and near the end it has:

    SPL_ME(SplObjectStorage, removeAll, arginfo_Object, 0)
    + SPL_ME(SplObjectStorage, removeCommon, arginfo_Object, 0)

    So there's already a removeAll.
    Read the doc patch. The description says it all, but if you still
    wonder then look at the example.
    In the example, substituting removeCommon by removeAll has exactly the
    same result:

    <?php
    $a = (object) 'a';
    $b = (object) 'b';
    $c = (object) 'c';

    $foo = new SplObjectStorage;
    $foo->attach($a);
    $foo->attach($b);

    $bar = new SplObjectStorage;
    $bar->attach($b);
    $bar->attach($c);

    $foo->removeAll($bar);
    var_dump($foo->contains($a));
    var_dump($foo->contains($b));

    gives

    bool(true)
    bool(false)


    Could you please explain the difference instead of telling me to RTFP?

    --
    Gustavo Lopes
  • Hannes Magnússon at Dec 19, 2010 at 3:24 pm

    On Sun, Dec 19, 2010 at 16:16, Gustavo Lopes wrote:
    On Sun, 19 Dec 2010 15:12:17 -0000, Hannes Magnússon
    wrote:
    On Sun, Dec 19, 2010 at 16:05, Gustavo Lopes <glopes@nebm.ist.utl.pt>
    wrote:
    On Sun, 19 Dec 2010 15:03:22 -0000, Hannes Magnússon
    wrote:
    On Sun, Dec 19, 2010 at 15:49, Gustavo Lopes <glopes@nebm.ist.utl.pt>
    wrote:
    On Sun, 19 Dec 2010 05:33:56 -0000, Matthew Turland
    wrote:
    Attached are source/test and manual patches for the addition of two
    methods to the SplObjectStorage class. These methods, removeCommon and
    removeUncommon, add support for the difference and intersection set
    operations, respectively.
    I'm sorry, I must be missing something because what exactly is the
    difference between removeCommon and removeAll?
    Is this just bikeshedding over the method names, or did you not bother
    reading the patches? (the doc patch even includes examples).
    I did, and near the end it has:

    SPL_ME(SplObjectStorage,  removeAll,   arginfo_Object,        0)
    +       SPL_ME(SplObjectStorage,  removeCommon,   arginfo_Object,     0)

    So there's already a removeAll.
    Read the doc patch. The description says it all, but if you still
    wonder then look at the example.
    In the example, substituting removeCommon by removeAll has exactly the same
    result:
    Haha. I was under the impression removeAll().. did what the method
    name says, sorry :P

    -Hannes
  • Matthew Turland at Dec 20, 2010 at 7:41 pm
    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.

    Regards,

    Matthew Turland
  • Matthew Turland at Dec 20, 2010 at 7:50 pm
    Sorry to flood the list, but I noticed that I left a stray reference
    to removeCommon in my amended patch. Attached the fixed version. My
    profound apologies.

    Regards,

    Matthew Turland
  • Gustavo Lopes at Dec 20, 2010 at 8:05 pm

    On Mon, 20 Dec 2010 19:41:29 -0000, Matthew Turland 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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedDec 19, '10 at 5:34a
activeDec 20, '10 at 8:05p
posts8
users3
websitephp.net

People

Translate

site design / logo © 2022 Grokbase