Re: [boost] [review] Last day for formal review for Sort library

Greetings, I do not intend to send a formal review but I nevertheless compiled and executed the provided test code, with clang 3.5. I added timings and confirmed that in the provided test case, this sort is significantly faster than std::sort() provided by the latest Linux Mint distribution. To the author, I would suggest two very minor issues : 1. There are compiler warnings that can easily be avoided (seen with '-Wunused-parameter' and '-Wold-style-cast'). 2. In string_sort_test.cpp, tests are performed using an array of 32 elements. 10 000 or more should make a more convincing test. I have also seen in the documentation that samples are available. Indeed, they are in git. However, I would appreciate samples in the documentation, without any useless I/O code (useless to understand how to use the sort functions). For example, taken from 'keyplusdatasample.cpp' : // --------------- Start of code snippet ------------------------ struct DATA_TYPE { int key; std::string data; }; struct lessthan { inline bool operator()(const DATA_TYPE &x, const DATA_TYPE &y) const { return x.key < y.key; } }; struct rightshift { inline int operator()(const DATA_TYPE &x, const unsigned offset) { return x.key >> offset; } }; std::vector array; integer_sort(array.begin(), array.end(), rightshift(), lessthan()); // --------------- End of code snippet ------------------------ Having concise code snippets like the above in the documentation is in my opinion the best way to have users quickly understand how the library must be used. I would also like to see one or two more example using "complex" data types. Consider for example : // --------------- Start of code snippet ------------------------ struct Md5Sum { uint8_t bytes[16]; bool operator<(const Md5Sum& md5) const { for (int k = 0; k < 16; k++) if (bytes[k] < md5.bytes[k]) return true; return false; } }; // --------------- End of code snippet ------------------------ After reading your documentation and the provided samples, I understand that I should use string_sort() after defining some get_char() and get_length() functions. Am I right ? Another example, sorting by birth data and then by name : // --------------- Start of code snippet ------------------------ struct Id { std::string name; time_t birth; bool operator<(const Id& id) const { return (birth < id.birth) || ((birth == id.birth) && (name < id.name)); } }; // --------------- End of code snippet ------------------------ Again, I understand that I must use string_sort and split the birth date into 8-bit components. Am I right ? These samples are important as there is probably much more people sorting structures than simple integers. Regards, Étienne ------ Message d'origine ------ De : "Edward Diener" À : "boost@lists.boost.org" Cc: "boost-users@lists.boost.org" Envoyé 19/11/2014 13:30:53 Objet : [boost] [review] Last day for formal review for Sort library

Today is the last day for the formal review of the Sort library by Steven Ross, which started November 10.

We have had some great discussions and reviews so far and I would like to thank everybody involved, but as always we need more formal reviews. If you need any delay for your review after today, please contact me at 'eldiener at tropicsoft dot com' and I will take your review into account if given within a reasonable period of time after today.

This message and any attachments are confidential and intended solely for the addressees. Any unauthorized modification, edition, use or dissemination is prohibited. If you have received this message by mistake, please notify us immediately. ATEME decline all responsibility for this message if it has been altered, deformed, falsified or even edited or disseminated without authorization.