Re: [boost] [locale] Formal review of Boost.Locale library -- final days

Subject: Re: [boost] [locale] Formal review of Boost.Locale library -- final days
From: Gevorg Voskanyan (v_gevorg_at_[hidden])
Date: 2011-04-23 01:52:38

Here's my formal review of the Boost.Locale library.

> Writing a  review
> ================
> 
> If you feel this is an interesting library,  then please submit your
> review to the developer list (preferably), or to the  review manager
> (me).
>
> Here are some questions you might want to answer  in your review:
> 
> - What is your evaluation of the design?

I find this library mostly well-designed for localization needs.
Points I like most:
- Being designed around std::locale system
- Great integration with I/O streams
- Concentrating mostly on ICU
- Yet having backend architecture to account for new possible backends as having 
them becomes reasonable
- Supporting char, wchar_t, char16_t and char32_t
- Using (and lobbying for) UTF-8 as default encoding

Points I don't like most:
- The boundary interface isn't attractive (in my opinion, shared by some other 
reviewers)
- Operator overloading is sometimes used in unintuitive ways. For example 
operator/ used to get the value for a specific period type

Observations:
The interface sometimes doesn't meet the expectations a Boost user could have 
from a Boost library. E.g. not accepting arbitrary ranges as arguments.
The overall design of the library takes into account ABI compatibility and 
ability to switch backends at runtime. Such concerns are not typical for Boost 
libraries so they have more freedom in some design decisions where Boost.Locale 
has no much choice. Which I personally don't find to be much of a problem and 
IMO shouldn't prevent acceptance into Boost. The author goes long ways to make 
Boost.Locale usable not only for regular Boost users but also for people who 
write *nix packages (including himself - CppCMS). This might see Boost.Locale 
gain more popularity, which can only have positive effect on the library and its 
evolution.

> - What is  your evaluation of the implementation?

I glanced over the sources but didn't give it the thorough examination it 
deserves. I generally find the implementation reasonable given the constraints 
above. I noticed lack of exception safety in some places and places where RAII 
should have been used. The implementation might benefit from a careful 
examination intended to find and fix such kind of problems. I also think that 
dynamic memory management is used too freely - in many cases their usage is 
unavoidable but in some cases they are. Having said that I appreciate that some 
efforts have already been put in that direction: I noticed several places where 
fixed-size buffers were used if data is small enough and dynamic allocation if 
it's larger.

More details at http://permalink.gmane.org/gmane.comp.lib.boost.devel/218352

> - What is your evaluation of the  documentation?

The documentation structure is good. It enables a new user to get started with 
the library quickly. The well-organized references help to find documentation 
for a specific facility in almost no time. I would however like to see more 
links within the text of the pages. There are many spelling and grammar issues 
in the documentation. Many of them have been already addressed to some extent by 
me and other reviewers, but it feels like there are still more of them left 
there. I agree that the documentation should be thoroughly scanned for such 
language issues by a native English speaker.
I also think there should be more examples.

More details at http://permalink.gmane.org/gmane.comp.lib.boost.devel/218117 and 
http://permalink.gmane.org/gmane.comp.lib.boost.devel/218343

> - What is your evaluation of the potential usefulness of the  library?

Very useful.

> - Did you try to use the library? With what compiler? Did you have  any
>   problems?

I have played with the library using number/ordinal formatting, date formatting, 
currency formatting, spellout, case conversion and boundary analysis features. I 
have not used encoding conversion, normalization, timezone/calendar, (message) 
formatting and collation features. I have used ICU backend only.

Environment:
FreeBSD 8.0
g++ 4.5.0 in C++03 mode
ICU 3.8.1 system-wide installation

What I used was intuitive and easy, except for boundary analysis as noted above.

I had some incorrect results most of which I believe are ICU issues.
Some which I'm not so sure about though:
- output of as::local_time was wrong by 2 hours; as::gmt was correct. Moreover, 
as::date printed the yesterday's date when the local time was 2:24 AM !
/bin/date shows correct local time, so I believe my system time zone settings 
are fine.
- "Note: not all locales provide rules for spelling numbers. In such a case the 
number would be displayed in decimal format." This was not the case:         
std::cout << as::spellout << 117 << std::endl; printed "one hundred and 
seventeen" under hy_AM.UTF-8 locale.

There were loads of "comma at end of enumerator list" warnings.

> - How much effort did you put into your evaluation? A  glance? A quick
>   reading? In-depth study?

Closer to in-depth study with more than 25 hours invested.

> - Are you knowledgeable  about the problem domain?

I am well aware of localization problems and approaches that can be used to 
tackle them. I have developed in-house solutions to address some localization 
needs on as-necessary basis.

> And finally, every review should answer this  question:
> 
> - Do you think the library should be accepted as a Boost  library?

Yes.

> Be sure to say this explicitly so that your other comments  don't
> obscure your overall opinion.
> -- 
> Chad Nelson
> Oak Circle  Software, Inc.

Gevorg