Exposing parameter types in a perfectly-forwarding function avoiding code repetition

Question

I have an annoying scenario where I need to defer the initialization of some object state and allow the user to construct one on demand. E.g.

// user code

context c;
// ...do something...
c.initialize_state(a, b, c);

// library code

class context
{
private:
    class state
    {
        state(A a, B b, C c);

        state(const state&) = delete;
        state(state&&) = delete;
    };

    std::optional<state> _state; // or `boost::optional`

public:
    template <typename... Xs>
    void initialize_state(Xs&&... xs) 
    {
        _state.emplace(std::forward<Xs>(xs)...);
    }
};

As you can see from the code above, the interface of context::initialize_state tells the user nothing about how to initialize context::_state. The user is forced to look at the implementation of initialize_state and then look at state::state to understand what should be passed to initialize_state.

I could change initialize_state to...

void initialize_state(A&& a, B&& b, C&& c) 
{
    _state.emplace(std::move(a), std::move(b), std::move(c));
}

...but this has a major drawback: there is code duplication with state::state, that needs to be manually maintained in case the argument types change.

Is there any way I can get the best of both worlds (DRY and user-friendly interface)? Note that state is not movable/copyable.

a minor detail: are you sure of the c++11 tag? AFAIK std::optional is c++17. — Picaud Vincent, May 16 '18 at 11:33
I think this post reads more like a criticism of variadic templates in general — AndyG, May 16 '18 at 11:38
It's a shame `initialize_state` is a function. If it were a type, you could only declare `initialize_state` and fully instantiate `initialize_state` in the header, and _define_ it elsewhere (in a source file for instance). — YSC, May 16 '18 at 11:38
@PicaudVincent: sorry, I'm using an internal C++11 replacement for `std::optional` in this particular project. — Vittorio Romeo, May 16 '18 at 11:39
I wouldn't really call this a "code duplication". This `initialize_state` function acts more like a façade for user. — user7860670, May 16 '18 at 11:41
Is `state` not movable/copyable because of some constraint on one of its members like `a`, `b` or `c`? I could see a scenario where we have a `StateCreator` class that eases things a bit. — AndyG, May 16 '18 at 11:41
@AndyG: yes, that's correct. `context` is also not movable itself for the same reason. — Vittorio Romeo, May 16 '18 at 11:44
Since it appears that `a` `b` and `c` are individually movable, it sounds like maybe `state` has some other internal members that prevent movability. It sounds like encapsulation is a good approach; pull out `A` `B` and `C` into another class that can be passed in. — AndyG, May 16 '18 at 11:47
Couldn't you have three type aliases and use those aliases in the constructor and `initialize_state` instead? Changing the aliases then changes the types for both functions. — cadaniluk, May 16 '18 at 11:47
Do you mean literally `void initialize_state(A&& a, B&& b, C&& c)` or should they be perfectly forwarded? — Passer By, May 16 '18 at 12:12

score 3 · Answer 1 · answered May 16 '18 at 11:56

The class state may not be copyable/movable, but it appears that A, B and C are. (So I'm assuming there is some other, internal data in state that prevents copyability/movability)

You can pull these members out into another class that can be injected into state. For lack of a better name, I'll call it state_args:

struct state_args
{
   explicit state_args(A a, B b, C c);
   A a_;
   B b_;
   C c_;
};

Which enables the following:

class context
{
private:
    class state
    {
        state(state_args args);

        state(const state&) = delete;
        state(state&&) = delete;
    };

    std::optional<state> _state; // or `boost::optional`

public:
    template<class STATE_ARGS, /*enable_if to ensure STATE_ARGS is indeed state_args*/>
    void initialize_state(STATE_ARGS&& internal_state) 
    {
        _state.emplace(std::forward<STATE_ARGS>(internal_state));
    }
};

I suggest `static_assert` over SFINAE and not all caps type name because ouch. — Passer By, May 16 '18 at 12:15

score 2 · Accepted Answer · answered May 16 '18 at 12:21

but this has a major drawback: there is code duplication with state::state, that needs to be manually maintained in case the argument types change.

This is a general problem with encapsulation. It's (definition) not DRY.

There is a way to preserve the relationship between the state constructor overloads and the interface of initialize_state, which is to use enable_if along with the is_constructible type trait.

class context
{
private:
    class state
    {
    public:
        state(A a, B b, C c);

        state(const state&) = delete;
        state(state&&) = delete;
    };

    std::optional<state> _state; // or `boost::optional`
public:
    template <typename... Xs>
    auto 
    initialize_state(Xs&&... xs) 
    -> 
    std::enable_if_t
    <
        // condition
        std::is_constructible<state, Xs...>::value, 

        // return type
        void
    >
    {
        _state.emplace(std::forward<Xs>(xs)...);
    }
};

score 2 · Answer 3 · answered May 16 '18 at 12:40

Probably creating more issues than solving, you might template your class:

template <typename ... Ts>
class context_impl
{
private:
    class state
    {
        state(Ts...);

        state(const state&) = delete;
        state(state&&) = delete;
    };

    std::optional<state> _state; // or `boost::optional`

public:
    void initialize_state(Ts&&... xs) 
    {
        _state.emplace(std::forward<Xs>(xs)...);
    }
};

using context = context_impl<A, B, C>;

As template are fixed by the class, void initialize_state(Ts&&... xs) has fixed signature (intellisense can show expected arguments for example).

Exposing parameter types in a perfectly-forwarding function avoiding code repetition

3 Answers3