Adding is_palindromic() algo to Boost.Algorithm

[zamazan4ik]

Hello. I implemented is_palindromic() algo for Boost.Algorithm.

The algorithm compares container from two sides. You can use custom comparator. Also supported Boost.Range. Complexity is O(N).

Using is easy: you should give two bidirectional or most powerful iterators for [begin, end) and you may give custom comparator.

Result is bool.

Maybe you have some additions to this algorithm?

Best regards, Zaitsev Alexander.

[mclow]

Needs tests ;-) Off the top of my head, it needs tests for: zero element input, single element, odd #, even #. That would be a good start.

The tests should exercise both the version that uses operator== and the one that takes a comparison functor explicitly.

[zamazan4ik]

I tested it on tests like that: zero element input, single element, odd and even. Also I tested on set of random strings. Should I add my tests to tests on GitHub?

[mclow]

Yes. The tests should be part of the pull request. Also, documentation.

[mclow]

You can get most of the documentation that you want by putting doxygen comments in the source, but you'll have to write a description, rationale, etc.

[mclow]

in your documentation, there should be "What should you use this for"? (so, real-world examples).

[zamazan4ik]

Can I use in tests C++14?

[mclow]

I would not if I were you; that would limit the people who run the tests. I don't see anything in the code you have that uses c++14.

[zamazan4ik]

You can't see it - it's not yet in the repo. It's my code for testing(using auto in arguments in lambda). Ok, I'll not use C++14. What about C++11? I think, it's widespread compiler.

[zamazan4ik]

Can you send me your config file for doxygen? Because i don't know about style of Boost's documentation.

[zamazan4ik]

I have done examples, tests and documentation. What should i do next?

[mclow]

This is looking good. Two comments: why call it is_palindromic, rather than just is_palindrome? That's a simpler name. What do you think?

Also, in the docs, you write:

The header file 'is_palindromic.hpp' contains four variants of a single algorithm, is_palindromic. The algorithm tests the sequence and returns true if the sequence is palindromic.

That doesn't really tell the reader anything. I think that this would be better:

The header file 'is_palindromic.hpp' contains four variants of a single algorithm, is_palindromic. The algorithm tests the sequence and returns true if the sequence is a palindrome; i.e, it is identical when traversed either backwards or frontwards.

(basically insert a definition of what a palindrome is)

[zamazan4ik]

Yes, i agree with your additions.

[zamazan4ik]

I wrote 'is_palindromic' because before aming of function i discuss about this with some other people. And first variant was 'is_palindromic'. But your variant is better, from my point of view. I don't know, why i didn't name function 'is_plaindrome' before your comment.

I rewrited name of function in the implementation, examples, tests, documents.

Something else?

[mclow]

Ok; I think this is ready to commit. However, I'll be committing it manually, because this isn't the right place for it. My personal repo is different from the boost one. This should be committed to https://github.com/boostorg/algorithm:develop

[zamazan4ik]

ok. For every next set of features should i create a new feature branch?