Skip to content
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

[Suggestion] ExtractFirstError meta function SHOULD be non-error safe #155

Open
SelcukAydi opened this issue May 17, 2023 · 2 comments
Open

Comments

@SelcukAydi
Copy link

I believe that a meta function should be safe to instantiate its internal type. For example, ExtractFirstError has a type member type alias only in the case of an error propagated. This forces the user to be sure that this meta function should be used in the case of an error to get its type member. However, when there is no error at all then this meta function's apply struct's recursive instantiation will be ill-formed. So a suggestion may be as the following:

Current code

struct ExtractFirstError {
  template <typename... Types>
  struct apply;

  template <typename Type, typename... Types>
  struct apply<Type, Types...> : public apply<Types...> {};

  template <typename ErrorTag, typename... ErrorParams, typename... Types>
  struct apply<Error<ErrorTag, ErrorParams...>, Types...> {
    using type = Error<ErrorTag, ErrorParams...>;
  };
};

Suggestion code

struct ExtractFirstError
{
    template <typename... Types>
    struct apply;

    template <typename Type>
    struct apply<Type>
    {
        using type = None;
    };

    template <typename Type1, typename Type2, typename... Types>
    struct apply<Type1, Type2, Types...> : public apply<Type2, Types...>
    {
    };

    template <typename ErrorTag, typename... ErrorParams>
    struct apply<Error<ErrorTag, ErrorParams...>>
    {
        using type = Error<ErrorTag, ErrorParams...>;
    };
};
@poletti-marco
Copy link
Contributor

Hi, the precondition for calling this ATM is that there is an error but i guess alternatively it could also be changed as you mention, the downside of that is that we wouldn't notice if this is accidentally called in a non-error case, but it's not a huge deal.

That said I'm surprised you were looking into this, this is internal Fruit code.
From a Fruit user point of view it doesn't matter right?
Or, is there a case where this would help / where a Fruit user would notice the difference?
That could be a stronger reason to prefer one or the other formulation.

@SelcukAydi
Copy link
Author

Calling a meta function may have a precondition in terms of internal code perspective. I totally agree. I just wanted to give a suggestion that follows the best practices. It is just very subjective.

From the perspective of the end-user, this does not matter obviously however, this meta folder might have been more reusable and agnostic. But again you say this is an internal code and I got no counter-argument :) Your code is already readable and easy to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants