Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make TypeHandler far more usable #4387

Closed
lousyphreak opened this issue Jan 7, 2024 · 3 comments
Closed

Make TypeHandler far more usable #4387

lousyphreak opened this issue Jan 7, 2024 · 3 comments
Assignees
Labels

Comments

@lousyphreak
Copy link

Is your feature request related to a problem? Please describe.
The way Typehandler is designed makes it very hard to add custom TypeHandlers that cover similar classes, or more specifically enums with std::enable_if.

Describe the solution you'd like
Add a second template parameter to Typehandler:

template <class T, class Enable = void>
class TypeHandler: public AbstractTypeHandler
// implementation...

This should not break any existing code, since the parameter is new and has a default.

Describe alternatives you've considered
I do not think that there are alternatives, but i would be glad to be told that i am wrong :)

Additional context
Adding the second parameter enables creating a TypeHandler that works for all enums:

template <typename EnumT>
class TypeHandler<EnumT, std::enable_if_t<std::is_enum_v<EnumT>>>
{
public:
    static std::size_t size()
    {
        return 1;
    }

    static void prepare(std::size_t pos, const EnumT& obj, AbstractPreparator::Ptr pPreparator)
    {
        poco_assert_dbg(!pPreparator.isNull());
        pPreparator->prepare(
            pos,
            reinterpret_cast<const typename std::underlying_type<EnumT>::type&>(obj));
    }

    static void bind(std::size_t pos, const EnumT& obj, AbstractBinder::Ptr pBinder, AbstractBinder::Direction dir)
    {
        poco_assert_dbg(!pBinder.isNull());
        pBinder->bind(
            pos,
            reinterpret_cast<const typename std::underlying_type<EnumT>::type&>(obj),
            dir);
    }

    static void extract(std::size_t pos, EnumT& obj, const EnumT& defVal, AbstractExtractor::Ptr pExt)
    {
        poco_assert_dbg(!pExt.isNull());
        if (!pExt->extract(
            pos,
            reinterpret_cast<typename std::underlying_type<EnumT>::type&>(obj)))
                obj = defVal;
    }
};

Adding that TypeHandler directly to poco might not be a good idea, but with the second template argument consumers of Poco can decide to do it on their own.

@matejk
Copy link
Contributor

matejk commented Jan 8, 2024

I think that similar approach could also be used with Poco::Dynamic::Any (#4363, #4341).

@aleks-f
Copy link
Member

aleks-f commented Feb 1, 2024

@matejk this is different because it's statically typed; Dynamic::Var allows for dynamic datatypes discovery and binding, but the flexibility comes with a performance price

@aleks-f aleks-f added this to the Release 1.14.0 milestone Feb 1, 2024
@aleks-f aleks-f added this to 1.14 Feb 1, 2024
@aleks-f aleks-f self-assigned this Feb 1, 2024
@matejk
Copy link
Contributor

matejk commented Feb 1, 2024

@aleks-f: Sorry for not being clear.

I was more referring to having partial specialisations for a group of types (like integers, enums, floats) instead of having specialisations for each type separately in Dynamic::Var.

@aleks-f aleks-f removed this from 1.14 Oct 30, 2024
@aleks-f aleks-f removed this from the Release 1.14.0 milestone Oct 30, 2024
@pocoproject pocoproject locked and limited conversation to collaborators Oct 30, 2024
@aleks-f aleks-f converted this issue into discussion #4749 Oct 30, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

3 participants