Re: [boost] radix sort

Subject: Re: [boost] radix sort
From: Evgeny Panasyuk (evgeny.panasyuk_at_[hidden])
Date: 2013-07-21 10:28:05

21.07.2013 16:29, Jeremy Murphy:

> Out of curiosity, I decided to tackle the problem and wrote an
> implementation of counting sort, which as you may know, is one way of
> implementing radix sort.  The code is not ready for a formal review but I
> would really appreciate feedback on it, as it is written with a view to
> inclusion in Boost.
> https://github.com/jeremy-murphy/integer-sort

Some thoughts after very quick look:

1. raising 2 to unsigned power via std::pow. just use (1 << i)

2. include directives of <cstring> and <climits> within namespace boost

3. There is "#ifdef __GXX_EXPERIMENTAL_CXX0X__" - one case uses lambda, 
second uses local class as functor.
First of all - there is no need to do both versions - just make one 
which is gcd of all target compilers.
Second, afaik - C++03 does not allow to use local classes as template 
arguments.

4. Code in main.cpp looks messier, try to partition it into logical blocks.

5. There is "assert(__first != __last);" at begin of algorithm - while 
common approach (for instance at STL) is to allow empty ranges. You may 
just do something like "if(__first == __last) return;"

> I'm particularly still uncertain about the interface (asking for forward
> AND reverse iterators seems annoying, however legitimate).  Thanks, cheers.

I think it is better just to take BidirectionalIterator - you always can 
use std::reverse_iterator in order to reverse it (however, while it 
simplifies code, it may incur additional overhead at dereferencing, so 
for widely used algorithm it is better to avoid it).

-- 
Evgeny Panasyuk