Re: [boost] Boost.Align review begins today

Subject: Re: [boost] Boost.Align review begins today
From: Mostafa (mostafa_working_away_at_[hidden])
Date: 2014-04-21 02:05:31

On Sun, 20 Apr 2014 22:52:31 -0700, Andrey Semashev  
<andrey.semashev_at_[hidden]> wrote:

> On Sunday 20 April 2014 21:46:34 Mostafa wrote:
>> 3. Implementation
>>    3.1) Why not reuse boost::static_unsigned_max in max_align?
>>    3.2) Why not the more readable
>> "boost::integer_traits<std::size_t>::const_max"
>>      instead of the less readable "~static_cast<std::size_t>(0)"?
>
> Since that was my suggestion, I'll comment on that. I'm aware of  
> Boost.Integer
> but still suggested the "~static_cast<std::size_t>(0)" variant to avoid  
> the
> dependency and integer_traits template instantiation cost.  
> integer_traits.hpp
> header is considerably heavier to include than the max_count_of.hpp. I  
> believe
> static_unsigned_max is not used in max_align.hpp for the same reasons.
>
> IMO the current code in Boost.Align is quite readable as it is and using
> Boost.Integer primitive wouldn't improve readability too much to justify  
> the
> costs.

Fair enough. Then that should be documented in the code. It's the first  
question that anyone familiar with Boost will ask when reviewing the code,  
and will help prevent future maintainers from "simplifying" it. (IMO,  
reinventing the wheel should require a good reason and that reason should  
be documented. Else some people will ask why and other people will be  
tempted to refactor code to use the existing wheel.)