Generating Actions: A Small Design Discussion
Here's a mildly interesting small-scale design choice.
I'm generating a set of actions corresponding to a set of states which are represented by monotonically-increasing enums. (And when I say generating, I mean it: this is actually code produced by a code generation tool I'm writing, not manually.) Each concrete instance implements the interface:class ITransmogrifierAction
{
public:
virtual ~ITransmogrifierAction();
virtual TestEnum type() const = 0;
virtual std::pair<Token,TestEnum> process(const TokenType& inVal) = 0;
};
and the array being filled -
std::array<std::unique_ptr<ITransmogrifierAction>, 5> m_actions;
- will end up being a lookup for state machine transitions.
The obvious STL way to do this is via generate_n.
class Filler
{
public:
std::unique_ptr<ITransmogrifierAction> operator()()
{
switch (m_n++)
{
case 0:
return std::make_unique<AlphaTransmogrifierAction>();
case 1:
return std::make_unique<BetaTransmogrifierAction>();
case 2:
return std::make_unique<GammaTransmogrifierAction>();
case 3:
return std::make_unique<DeltaTransmogrifierAction>();
default:
return std::make_unique<EpsilonTransmogrifierAction>();
}
}
private:
int m_n = 0;
};
std::generate_n(m_actions.begin(), 5, Filler{});
However, there's another way which draws on the type() call in the class:
auto l
= [&](std::unique_ptr<ITransmogrifierAction> &&inAction) {
m_actions[ auto tp = static_cast<int>(inAction->type()); static_cast<int>(tp)] = std::move(inAction);
};
l(std::make_unique<AlphaTransmogrifierAction>());
l(std::make_unique<BetaTransmogrifierAction>());
l(std::make_unique<GammaTransmogrifierAction>());
l(std::make_unique<DeltaTransmogrifierAction>());
l(std::make_unique<EpsilonTransmogrifierAction>());
This is, I grant you, very non-STL. But it does guarantee that the lookup by type which will be used on the completed array is used in filling it, and it's rather shorter than the generate_n version.
The minimum length of any implementation of this logic is five statements: that's because there's no intrinsic way to get from the enum value to the action, so you need five literal creation calls for the five types of action. The second implementation adds one line to simplify the individual calls.
The issue is that generate() and generate_n() are wonderful when there are substantive and logical transformations which can be used to derive each of the iterated insertions. But when the link is essentially arbitrary, all you do is trade a sequential set of calls to do insertions for a big switch statement to do the determination of what is to be generated.
The advantage of the generate_n model is that it puts the messiness into a separate class which can be, if necessary, in a different file or even module. If this is being slotted into an already busy function it is less likely to contribute to cognitive overload from the length.
There's another version of this, though, which uses a factory. The linear code, in that case, looks like:
auto l = [&](std::unique_ptr<ITransmogrifierAction>&& inAction) { m_actions[static_cast<int>(inAction->type())] = std::move(inAction); };
l(inFactory.create(TestEnum::Alpha));
l(inFactory.create(TestEnum::Beta));
l(inFactory.create(TestEnum::Gamma));
l(inFactory.create(TestEnum::Delta));
l(inFactory.create(TestEnum::Epsilon));
and the filler now looks like:
class Filler
{
public:
Filler(const ITransmogrifierActionFactory& inFactory): m_factory(inFactory) { }
std::unique_ptr<ITransmogrifierAction> operator()()
{
auto rval = m_factory.create(m_enum);
if (m_enum != TestEnum::Epsilon)
m_enum = static_cast<TestEnum>(static_cast<int>(m_enum) +1);
return rval;
}
private:
const ITransmogrifierActionFactory& m_factory;
TestEnum m_enum = TestEnum::Alpha;
};
In this case, the generate_n model is clearly superior - shorter, more purely logic rather than a data sequence - the result of the classical extra layer of indirection. It uses brute force to increment the enum, but as the enum is guaranteed by the rest of the code generation program to begin at zero and increase monotonically, it's entirely safe if slightly ugly. (The explicit calls to make_unique occur when the main class uses no factory and manages its own contents entirely, meant for very short cases where encapsulation in a single file might outweigh the other downsides of having the extra details visible in a single class. This is thus another indirect plug for the SRP: managing an object's creation in the same place that you manage its use is a violation of the SRP once the complexity of construction becomes great enough.)
That guard:
if (m_enum != TestEnum::Epsilon)
isn't really needed: the factory code will handle out-of-bounds calls, to begin with, and because the code is automatically generated it is, at least initially, guaranteed that the number of calls in generate_n matches exactly the valid set of enums.
So we can simplify it, moving to a closure now that it has become so much more simple, to
int e = 0;
std::generate_n(m_actions.begin(), 5,
[&]() -> std::unique_ptr<ITransmogrifierAction>
{
return inFactory.create(static_cast<TestEnum>(e++));
});
In addition, the code as it stands is a frame to be filled in. Auto-generation can give you the structures, but the real details of the implementation come from how the actions are defined after that first step has been taken. That sequence of calls to generate the actions would almost never be that clean -- typically you would want to pass different resources into the different actions -- which is exactly where you really want a factory to manage the ugliness.
However, the original model works well for stateless actions, where you toggle between different choices of pure logic based on the current state, and it's worth keeping that use case in mind where that is a natural model. In that case the constructors are all trivial and there is little point in encapsulating the creation in a separate class.
Comments
Post a Comment