Re: [boost] radix sort

Subject: Re: [boost] radix sort
From: Jeremy Murphy (jeremy.william.murphy_at_[hidden])
Date: 2013-07-21 21:37:03

On 22 July 2013 00:28, Evgeny Panasyuk <evgeny.panasyuk_at_[hidden]> wrote:

> 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
>
>
> ______________________________**_________________
> Unsubscribe & other changes: http://lists.boost.org/**
> mailman/listinfo.cgi/boost<http://lists.boost.org/mailman/listinfo.cgi/boost>
>

Thanks for catching my stupid mistakes!  I've made all the changes you
recommended except that I'm still hedging my bets on C++11.  I guess I'll
ask Marshall Clow if there is a policy for inclusion into the Algorithm
library.

Cheers.

Jeremy