The sutfcpplib from devatrun

sutfcpplib's Issues

Inconsistency with end iterators in API

I wanna talk about code_unit_count/code_point_count functions.
(as I started to dive into your library, estimating the length was point of interest for me).

Then I found couple of

if constexpr (is_native_string_v<type_t>)
        --end;

and felt bad about it, so I can have unexpected result when passing iterator pair by myself (STL taught me to use end behind the end, while you code expects different meaning).

Don't test for C++version, test for features

#if !(defined(_MSC_VER) && _MSC_VER >= 1910 && ((defined(_MSVC_LANG) && _MSVC_LANG > 201402)) || (__cplusplus > 201402))
#error "Library SUTFCPP requires a compiler that supports C++ 17!"
#endif

I found such a check in your code.
I think it's much better to check for library feature macros and compiler feature macros for what you exactly used.

I had a lot of problems in my past, checking for __cplusplus and then having issues in Linux, when compiler and STL version are not in sync (imagine using clang 10 with libstdc++6 for example).

Reduce SFINAE parts in favor of 'if constexpr'

I saw you already used 'if contexpr'. So for me logic
of

////////////////////////////////////////////////////////////////////////////////////////////////////
template<typename char_t, std::enable_if_t<std::is_same_v<char_t, char> || std::is_same_v<char_t, char8s_t>, int>>
constexpr uint_t code_unit_count(uint_t cp) noexcept
{
    if (cp < 0x80)
        return 1;
    if (cp < 0x800)
        return 2;
    if (cp < 0x10000)
        return 3;

    return 4;
}



////////////////////////////////////////////////////////////////////////////////////////////////////
template<typename char_t, std::enable_if_t<std::is_same_v<char_t, char16_t> || (std::is_same_v<char_t, wchar_t> && sizeof(wchar_t) == sizeof(char16_t)), int>>
constexpr uint_t code_unit_count(uint_t cp) noexcept
{
    return cp < 0x10000 ? 1 : 2;
}



////////////////////////////////////////////////////////////////////////////////////////////////////
template<typename char_t, std::enable_if_t<std::is_same_v<char_t, char32_t> || (std::is_same_v<char_t, wchar_t> && sizeof(wchar_t) == sizeof(char32_t)), int>>
constexpr uint_t code_unit_count(uint_t /*cp*/) noexcept
{
    return 1;
}

Would be much clearer in form of

template<typename char_t> // i wish some char concept here, but no C++20, oh well. 
constexpr uint_t code_unit_count(uint_t cp) noexcept
{
   if constexpr (sizeof(char_t) == 1) {
    if (cp < 0x80)
        return 1;
    if (cp < 0x800)
        return 2;
    if (cp < 0x10000)
        return 3;

    return 4;
    } else if constexpr (sizeof(char_t) == 2) {
      return cp < 0x10000 ? 1 : 2;
    } else {
      return 1;
    }
}

or something.

If you don't like methods getting too long, then at least make you own traits or something.

Overall I wish you can provide concepts version instead of sfinae (so I can clearly understand API by signature). Docs can do that too, but I am a fan of self-documented code.

devatrun / sutfcpplib Goto Github PK

sutfcpplib's People

Contributors

Stargazers

Watchers

sutfcpplib's Issues

Inconsistency with end iterators in API

Don't test for C++version, test for features

Reduce SFINAE parts in favor of 'if constexpr'

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent