Hello List! Artyom wrote:
I'm waiting for your inputs
First I have to congratulate the author that he tries to fill a dark hole of current c++. I am afraid it is impossible to make such a library optimal for everyone. But there are many parts which are really great: - there are already many features - case conversion, case folding and normalization is done nice - as::spellout and so on is convenient - message formatting is like stated very powerful - std::string is supported (no new string type introduced) - but others are supported to - very good integration in iostreams and facets The overall design fits very well into C++ and boost. But for boundary analysis I have other expections: I would rather expect an iterator similar to boost::tokenizer http://www.boost.org/doc/libs/1_40_0/libs/tokenizer/index.html which lets me iterate over characters, words and sentences. The design of your boundary analysis seem to be very complicated and because of the manual use of offsets very error prone. There is a part where I see intersection to existing boost libraries: - date formatting, especially the ftime functionality are already supported by boost::date_time, see: http://www.boost.org/doc/libs/1_40_0/doc/html/date_time/date_time_io.html#da... - time zones, also already in boost::date_time however I find your way better that you use std::set<std::string> and not std::vector<std::string> like in the Time Zone Datebase of boost::date: http://www.boost.org/doc/libs/1_40_0/doc/html/date_time/local_time.html#date... On the other hand there are parts inside boost which should use boost::locale: - boost::regex (seems like it is currently using libicu directly?) - boost::format Some remarks about the doxygen documentation: http://cppcms.sourceforge.net/boost_locale/docs/doxy/html/ - It should read #include <boost/locale/generator.hpp> instead of #include <generator.hpp> (and all the others). - impl() should be private and should not be documented. "Do not use this" is not very useful. - Documentation missing for operator== in timezone: Does it compare object identity or id? Now I will discuss the problems in the order of the tutorial: http://cppcms.sourceforge.net/boost_locale/docs/ - I did not find a list of all available facets. How can I get back to the defaults (all facets activated) after some are turned off? == Collation == - There should be some explanation of std::use_facet or facet in general because it is a very seldom used feature of the C++ standard. Thats because without something like boost.locale it is barely useable. - loc was used in use_facet<collator<wchar_t> >(loc).compare(collator_base::secondary,a,b); but not defined before (inside Collation chapter). Better you use locale(). The same problem with some_locale and some_level. Give working examples whenever it is easily possible. - You wrote the comment: // Now strings uses default system locale for string comparison Does it means that it uses the default system locale ("C"), the locale when you call gen(""); or the default which was set before with std::locale::global()? I would suggest some glossary, like in boost::filesystem: http://www.boost.org/doc/libs/1_40_0/libs/filesystem/doc/reference.html#Defi... == Number, Time and Currency == - What does "as" mean? Is this just meant as the english word as? - Negative Numbers as currency do not work as expected. The output of the program with a de_AT locale is: -€ 14,00 but I would expect € -14,00 - Parsing has the same problem, but even worse the varialbe contains a completely wrong number afterwards! Parsing € -14,00 leads that the variable contains 11027 afterwards! The library really should throw an exception if there are parsing errors. - You did not mention that cout.imbue() or cin.imbue() must be called first in the beginning of the section. Just setting the global locale (or default locale?) is not enough. - as::ordinal did not work. In the locale de_AT I would expect << boost::locale::as::ordinal << 1 to output "1.". But it just outputs "1". The rule is simple: just add a "." for every number. - as::spellout (as stated above) is really convenient but I am missing a spellout for ordinal numbers (first, second, third, fourth or in german erste, zweite, dritte, vierte) - I missed the place where it is stated that e.g. boost::locale::as::currency_iso needs a boost::locale::as::currency before. - boost::locale::as::currency_iso does not work: std::cout << boost::locale::as::currency << boost::locale::as::currency_iso << 1523.45 << std::endl; € 1.523,45 std::cout << boost::locale::as::currency_iso << 1523.45 << std::endl; 1523.45 But it should read: 1.523,45 EUR - Why is the type double used for time? You should use time_t or TimeType instead. double is used in the example, but also in offset_from_gmt. - Also for time it should be more clear that e.g. time_long is a additional modificator to time. - gmt seems to modificate both. time_zone also modificated both, even though it has the prefix "time_". All others with that prefix only modificate time formatting. Maybe you should avoid prefixing and use a namespace instead? == Message Formatting == - typing mistakes: signle -> single boost::locale::message boost::locale::translate(const char*, const char*, int) <- last int missing - as::domain did not work, when using: std::cout << boost::locale::as::domain("foo") << boost::locale::translate("foo") << std::endl; it results to the compiler error: /usr/src/locale/libs/locale/../../boost/locale/message.hpp: In function ?std::basic_ostream<CharType, std::char_traits<_CharT> >& boost::locale::as::det ails::operator<<(std::basic_ostream<CharType, std::char_traits<_CharT> >&, const boost::locale::as::details::set_domain&) [with CharType = char]?: /usr/src/locale/libs/locale/examples/h.cpp:25: instantiated from here /usr/src/locale/libs/locale/../../boost/locale/message.hpp:391: error: no matching function for call to ?use_facet(std::locale)? - Why is wchar_t used even though it is assigned to a string? std::string msg = translate("Do you want to open the file?").str<wchar_t>(some_locale) - The datatype "message" in send_to_all is not explained? What is "ms"? - " missing and translate misspelled in: cout << format(tranlsate("You have 1 file in the directory",You have {1} files in the directory",files)) % files << endl; - gettext, ngettext are exposed into global namespace when <boost/locale.hpp> is included! == Code-page conversions == - Please give an example for code converter. == Boundary analysis == - Spelling: indx should be index? - first test example does not output any character (only line breaks) - Like above said, shouldn't it be a iterator interface? == Info == - misspelling: tranlsate - std::locale::name (or better std::locale().name()) just returns *, even german locals are set. I would expect that your library sets a name? - How is info meant to be used? ~info is protected, so it seems like that the object should not be instanciated myself? == Multiple Locales == - Do you mean gen.get() in: std::locale ar=get("ar_EG"); Why is it not cached in that case? == general == - What is your evaluation of the potential usefulness of the library? indispensable - Did you try to use the library? With what compiler? gcc (Debian 4.3.2-1.1) 4.3.2 ICU 3.6-2etch3 - How much effort did you put into your evaluation? about 6 hours - Are you knowledgeable about the problem domain? yes You have done good work and I am sure that it has the opportunity to get the standard for localization efforts. Keep at it! best regards Markus Raab -- http://www.markus-raab.org | Without a good library, most interesting -o) | tasks are hard to do in C++; but given a Kernel 2.6.24-1-a /\ | good library, almost any task can be made on a x86_64 _\_v | easy. -- Bjarne Stroustrup