Bug 141321 – Add Vector::removeFirstMatching() / removeAllMatching() methods taking lambda functions

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Chris Dumez]

Add Vector::removeFirstIf() / removeAllIf() methods taking lambda functions to match the element(s) to remove.

[Chris Dumez]

Created attachment 246149 [details]
Patch

[WebKit Commit Bot]

Attachment 246149 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:470:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:471:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:472:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:476:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:478:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:480:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:483:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:486:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:495:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:496:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:497:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:501:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:503:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:505:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:510:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:513:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:516:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 17 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 246163 [details]
Patch

[Anders Carlsson]

Comment on attachment 246163 [details]
Patch

I don't like the name removeAllIf; it sounds like it'll clear the vector if the lambda returns true.

[Chris Dumez]

(In reply to comment #4)
> Comment on attachment 246163 [details]
> Patch
> 
> I don't like the name removeAllIf; it sounds like it'll clear the vector if
> the lambda returns true.

No argument there. We need to find better names for removeFirst(value) / removeFirstIf(lambda), removeAll(value) / removeAllIf(lambda). Do you have any suggestion?

[WebKit Commit Bot]

Attachment 246163 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:470:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:471:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:472:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:476:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:478:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:480:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:483:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:486:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:495:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:496:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:497:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:501:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:503:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:505:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:510:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:513:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:516:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 17 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

I have the following naming proposal:
- removeAt(index)
- removeSingle(value) // first occurrence
- removeSingleIf(lambda) // first match
- remove(value) // every occurrence
- removeIf(lamba) // every match

Any thoughts?

[Chris Dumez]

(In reply to comment #7)
> I have the following naming proposal:
> - removeAt(index)
> - removeSingle(value) // first occurrence
> - removeSingleIf(lambda) // first match
> - remove(value) // every occurrence
> - removeIf(lamba) // every match
> 
> Any thoughts?

Darin, what do you think about this naming? Do you have a better proposal?

[Darin Adler]

I am trying to find some prior art. I think I prefer "First" to "Single" but there is something that rubs me the wrong way about using "if" to mean "based on a predicate", because "if" sounds like it’s the entire function that’s conditional, not just something the function does.

I would never say “remove all if equal to 5”, I would say “remove all that are equal to 5” or “remove all if there was a problem”.

I am trying to find some prior art on naming functions that take predicates. Even better if it was in a project that used a sort of “literate” naming style reminiscent to the one from Cocoa, which inspires a lot of our function naming in WebKit.

[Darin Adler]

Cocoa uses the words “using block” and “passing test” in their NSArray functions. Neither seems terse enough for our purposes.

[Chris Dumez]

(In reply to comment #9)
> I am trying to find some prior art. I think I prefer "First" to "Single" but
> there is something that rubs me the wrong way about using "if" to mean
> "based on a predicate", because "if" sounds like it’s the entire function
> that’s conditional, not just something the function does.
> 
> I would never say “remove all if equal to 5”, I would say “remove all that
> are equal to 5” or “remove all if there was a problem”.
> 
> I am trying to find some prior art on naming functions that take predicates.
> Even better if it was in a project that used a sort of “literate” naming
> style reminiscent to the one from Cocoa, which inspires a lot of our
> function naming in WebKit.

Well, STL has remove_if:
http://en.cppreference.com/w/cpp/algorithm/remove

I agree that removeAllIf() is confusing, which is why my latest proposal is removeIf().

[Chris Dumez]

For the lambda function, we could also use a method name with "Matching" in the name, e.g.:
removeFirstMatching(lambda) / removeAllMatching(lambda)

[Chris Dumez]

Created attachment 246224 [details]
Patch

[WebKit Commit Bot]

Attachment 246224 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:470:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:471:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:472:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:476:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:478:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:480:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:483:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:486:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:495:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:496:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:497:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:501:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:503:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:505:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:510:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:513:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Vector.cpp:516:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 17 in 23 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 246224 [details]
Patch

I like these “matching” names. And they are nice and long and easy to globally replace if we get another naming idea.

[Anders Carlsson]

(In reply to comment #15)
> Comment on attachment 246224 [details]
> Patch
> 
> I like these “matching” names. And they are nice and long and easy to
> globally replace if we get another naming idea.

I agree.

[Chris Dumez]

(In reply to comment #16)
> (In reply to comment #15)
> > Comment on attachment 246224 [details]
> > Patch
> > 
> > I like these “matching” names. And they are nice and long and easy to
> > globally replace if we get another naming idea.
> 
> I agree.

\o/

[WebKit Commit Bot]

Comment on attachment 246224 [details]
Patch

Clearing flags on attachment: 246224

Committed r179791: <http://trac.webkit.org/changeset/179791>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.