One SkTSearch to rule them all. Allow key to be of different type than the array.

One SkTSearch to rule them all. Allow key to be of different type than the array.

R=bungeman@google.com

Committed: https://code.google.com/p/skia/source/detail?r=9182

[bsalomon, 2013-05-16 21:03:58]

updated to make the op < variant call the generic T,K,functor version directly.

[bungeman-skia, 2013-05-16 22:00:39]

Some possible cleanup.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h
File include/core/SkTSearch.h (right):

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:73: class SkTLessFunctionToFunctorAdaptor {
If this were a struct there would be no need for the 'public' on the next line.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:83: return SkTSearch<T, T, Functor>(base, count,
target, elemSize, functor);
I think this can just be

return SkTSearch(base, count, target, elemSize, Functor());

and the previous line removed. All of the function template parameters should be
inferred.

Actually the line

return SkTSearch(base, count, target, elemSize,
SkTLessFunctionToFunctorAdaptor<T, LESS>());

appears to be 97 characters (after adding the four leading spaces). This allows
this function to be one line.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:88: public:
nit: This is a struct, so public is already implied.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:96: return SkTSearch<T, T, SkTLessFunctor<T> >(base,
count, target, elemSize, functor);
This line can probably be

return SkTSearch(base, count, target, elemSize, SkTLessFunctor<T>());

and the previous line removed.

I wonder if this helps with the 'const' issues, since the compiler will be the
one figuring out the types.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:101: class SkTLessFunctionToPtrFunctorAdaptor {
If this were a struct there would be no need for the 'public' on the next line.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:112: return SkTSearch<T*, T*, Functor>(base, count,
target, elemSize, functor);
Likewise, this line can probably be

return SkTSearch(base, count, target, elemSize,
SkTLessFunctionToPtrFunctorAdaptor<T, LESS>());

for a single line (exactly 100 columns with the four leading spaces).

[bsalomon, 2013-05-17 14:24:30]

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h
File include/core/SkTSearch.h (right):

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:73: class SkTLessFunctionToFunctorAdaptor {
On 2013/05/16 22:00:39, bungeman1 wrote:
> If this were a struct there would be no need for the 'public' on the next
line.

Done.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:83: return SkTSearch<T, T, Functor>(base, count,
target, elemSize, functor);
On 2013/05/16 22:00:39, bungeman1 wrote:
> I think this can just be
> 
> return SkTSearch(base, count, target, elemSize, Functor());
> 
> and the previous line removed. All of the function template parameters should
be
> inferred.
> 
> Actually the line
> 
> return SkTSearch(base, count, target, elemSize,
> SkTLessFunctionToFunctorAdaptor<T, LESS>());
> 
> appears to be 97 characters (after adding the four leading spaces). This
allows
> this function to be one line.

Done.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:88: public:
On 2013/05/16 22:00:39, bungeman1 wrote:
> nit: This is a struct, so public is already implied.

Done.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:96: return SkTSearch<T, T, SkTLessFunctor<T> >(base,
count, target, elemSize, functor);
On 2013/05/16 22:00:39, bungeman1 wrote:
> This line can probably be
> 
> return SkTSearch(base, count, target, elemSize, SkTLessFunctor<T>());
> 
> and the previous line removed.
> 
> I wonder if this helps with the 'const' issues, since the compiler will be the
> one figuring out the types.

Done. I'll leave fighting the const battle for another day ( or at least another
changelist).

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:101: class SkTLessFunctionToPtrFunctorAdaptor {
On 2013/05/16 22:00:39, bungeman1 wrote:
> If this were a struct there would be no need for the 'public' on the next
line.

Done.

https://codereview.chromium.org/15070011/diff/15001/include/core/SkTSearch.h#...
include/core/SkTSearch.h:112: return SkTSearch<T*, T*, Functor>(base, count,
target, elemSize, functor);
On 2013/05/16 22:00:39, bungeman1 wrote:
> Likewise, this line can probably be
> 
> return SkTSearch(base, count, target, elemSize,
> SkTLessFunctionToPtrFunctorAdaptor<T, LESS>());
> 
> for a single line (exactly 100 columns with the four leading spaces).

Actually only 99!

[bungeman-skia, 2013-05-17 14:36:05]

lgtm

[bsalomon, 2013-05-17 19:05:39]

Committed patchset #5 manually as r9182 (presubmit successful).