Skip to main content
added 473 characters in body
Source Link
Incomputable
  • 9.7k
  • 3
  • 34
  • 73

Code Review

  • Relatively low level

    Usually there is a standard library algorithms for most of uses

  • Pass by value

    With std::move() it would be reasonable, but passing by lvalue will incur a redundant copy.

  • Undocumented/unclear assumptions

    One of them is that only palindromes of length 3 and higher can be palindromes.

  • Verbose control flow

    One could immediately return after seeing that it is not palindrome.


I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As people in the comments suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    using std::begin;
    using std::end;
    auto half_distance = std::distance(begin(container), 
                                       end(container)) / 2;
    return std::next(begin(container), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    using std::cbegin;
    using std::crbegin;
    return std::equal(cbegin(container), middle(container), 
                      crbegin(container));
}

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As people in the comments suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    using std::begin;
    using std::end;
    auto half_distance = std::distance(begin(container), 
                                       end(container)) / 2;
    return std::next(begin(container), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    using std::cbegin;
    using std::crbegin;
    return std::equal(cbegin(container), middle(container), 
                      crbegin(container));
}

Code Review

  • Relatively low level

    Usually there is a standard library algorithms for most of uses

  • Pass by value

    With std::move() it would be reasonable, but passing by lvalue will incur a redundant copy.

  • Undocumented/unclear assumptions

    One of them is that only palindromes of length 3 and higher can be palindromes.

  • Verbose control flow

    One could immediately return after seeing that it is not palindrome.


I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As people in the comments suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    using std::begin;
    using std::end;
    auto half_distance = std::distance(begin(container), 
                                       end(container)) / 2;
    return std::next(begin(container), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    using std::cbegin;
    using std::crbegin;
    return std::equal(cbegin(container), middle(container), 
                      crbegin(container));
}
added 88 characters in body
Source Link
Incomputable
  • 9.7k
  • 3
  • 34
  • 73

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As @hoffmalepeople in the comments suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    using std::begin;
    using std::end;
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(container), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    returnusing std::equal(cbegin;
    using std::crbegin;
    return std::equal(cbegin(container), middle(container), 
                      std::crbegin(container));
}

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As @hoffmale suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    return std::equal(std::cbegin(container), middle(container), 
                      std::crbegin(container));
}

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As people in the comments suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    using std::begin;
    using std::end;
    auto half_distance = std::distance(begin(container), 
                                       end(container)) / 2;
    return std::next(begin(container), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    using std::cbegin;
    using std::crbegin;
    return std::equal(cbegin(container), middle(container), 
                      crbegin(container));
}
added 597 characters in body
Source Link
Incomputable
  • 9.7k
  • 3
  • 34
  • 73

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As @hoffmale suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    return std::equal(std::cbegin(container), middle(container), 
                      std::crbegin(container));
}

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.

I've actually answered one of these before. I wouldn't say it is canonical though. Here is what I've got:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

bool is_palindrome(const std::string& str) {
    return std::equal(str.cbegin(), middle(str), str.crbegin());
}

Advantages

  • Iterators

    This will allow easy transition to some other type

  • Standard algorithms

    No reinvention of a wheel, tested and updated by people who (usually) know what they're doing. Very condensed, yet clean code.


As @hoffmale suggested, one can easily generalize that:

template <typename Container>
constexpr auto middle(const Container& container)
{
    auto half_distance = std::distance(std::begin(container), 
                                       std::end(container)) / 2;
    return std::next(container.begin(), half_distance);
}

template <typename Container>
bool is_palindrome(const Container& container) {
    return std::equal(std::cbegin(container), middle(container), 
                      std::crbegin(container));
}
deleted 179 characters in body
Source Link
Incomputable
  • 9.7k
  • 3
  • 34
  • 73
Loading
Source Link
Incomputable
  • 9.7k
  • 3
  • 34
  • 73
Loading