Jim Apple has posted comments on this change. Change subject: Avoid std::function when possible. ......................................................................
Patch Set 1: > > I do not find it more readable, but I find it to be readable > enough; I think the readability delta is a good price to pay to > avoid boxing up a lambda. > > Could you be more specific about the drawbacks? We've spoken about > heap allocation; personally I don't think that's an issue because > it's not unusual, but perhaps that's a dealbreaker for you. > Otherwise I know you consider it 'overkill', but I don't yet know > why. Perhaps there's something about this class in particular that > makes it a bad choice for std::function? It is another abstraction layer between the caller and the function they passed in. With the code as it is now, they may have to understand the subtleties of no only anonymous functions but also std::function, a task that, as the slides and talk demonstrate, was too difficult in the corner cases for even the standards committee and will have to get fixed up in C++17. I don't know of a corner case yet that is going to bite us, but I'd rather not find one the hard way. -- To view, visit http://gerrit.cloudera.org:8080/5230 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9dc034c2e094ea7f87f78d8d9101a71d8d2e295 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-HasComments: No
