> On April 12, 2013, 9:21 p.m., Andrew Stitcher wrote: > > trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp, line 346 > > <https://reviews.apache.org/r/10436/diff/1/?file=280864#file280864line346> > > > > I think there's more code you can remove - getMatch() and the stuff it > > uses transitively too.
This is used by class Matcher and in function bind(). getMatch() should not be used in class Matcher - it's looking for key-value pairs on a per-message basis. In bind() the values could be found and set as bools in the binding. Then getMatch only checks bools. That said, the issue that is being addressed here is self test and is not a license to do performance fixes. > On April 12, 2013, 9:21 p.m., Andrew Stitcher wrote: > > trunk/qpid/cpp/src/tests/HeadersExchangeTest.cpp, line 169 > > <https://reviews.apache.org/r/10436/diff/1/?file=280865#file280865line169> > > > > Extra whitespace here, which hides the strange layout - put each > > section terminated by ";" on its own line Ok > On April 12, 2013, 9:21 p.m., Andrew Stitcher wrote: > > trunk/qpid/cpp/src/tests/HeadersExchangeTest.cpp, line 175 > > <https://reviews.apache.org/r/10436/diff/1/?file=280865#file280865line175> > > > > Here too ok > On April 12, 2013, 9:21 p.m., Andrew Stitcher wrote: > > trunk/qpid/cpp/src/tests/MessageUtils.h, line 104 > > <https://reviews.apache.org/r/10436/diff/1/?file=280866#file280866line104> > > > > createMessage() might be a sufficient name - This is actually creating > > a Message *with* specified application headers. I tried that. It then conflicts with the first createMessage() in the file. - Chug ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10436/#review19108 ----------------------------------------------------------- On April 12, 2013, 9:02 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10436/ > ----------------------------------------------------------- > > (Updated April 12, 2013, 9:02 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Description > ------- > > Code refactor for AMQP 1.0 added new header match logic for headers exchange > but did not update self tests to use it. This patch slightly refactors the > headers exchange logic to create a testable match function. Then it modifies > the self test to call the new function. A single new self test is added to > prove the match four sizes of int and four sizes of uint with each other. > > > This addresses bug QPID-4720. > https://issues.apache.org/jira/browse/QPID-4720 > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1467440 > trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1467440 > trunk/qpid/cpp/src/tests/HeadersExchangeTest.cpp 1467440 > trunk/qpid/cpp/src/tests/MessageUtils.h 1467440 > > Diff: https://reviews.apache.org/r/10436/diff/ > > > Testing > ------- > > The whole object of the patch is test. > > > Thanks, > > Chug Rolke > >
