clean-up design of the command handling patterns (#210)

this was a classical example of a muddled and messed-up design,
driven just by the fact that I wanted to "spare" some functions,
with the net effect of writing more functions, plus a proxy class
plus create a lot of confusion for the reader.

This was easy to resolve though, once I resorted to the
general adivice to make public interface methods final,
make the extension ponts protected and never
to chain two extension points
This commit is contained in:
Fischlurch 2016-01-22 15:25:08 +01:00
parent eaa12499f3
commit 005e665c13
10 changed files with 131 additions and 124 deletions

View file

@ -90,6 +90,15 @@ namespace control {
, LUMIERA_ERROR_UNBOUND_ARGUMENTS);
}
void
___check_canUndo (const Command *handle)
{
REQUIRE (handle);
if (!handle->canUndo())
throw error::State ("Lifecycle error: command has not yet captured UNDO information"
, LUMIERA_ERROR_UNBOUND_ARGUMENTS);
}
}
@ -368,26 +377,27 @@ namespace control {
ExecResult
Command::undo ()
{
___check_notBottom (this,"Un-doing");
HandlingPattern const& defaultPattern
= HandlingPattern::get (getDefaultHandlingPattern());
return exec (defaultPattern.howtoUNDO());
}
ExecResult
Command::exec (HandlingPattern const& execPattern)
{
___check_notBottom (this,"Invoking");
___check_isBound (this);
string cmdName{*this};
CommandImpl& thisCommand (_Handle::impl());
return execPattern.invoke (thisCommand, cStr(*this));
return execPattern.exec (thisCommand, cmdName);
}
ExecResult
Command::undo (HandlingPattern const& execPattern)
{
___check_notBottom (this,"UNDOing");
___check_canUndo (this);
string cmdName{*this};
CommandImpl& thisCommand (_Handle::impl());
return execPattern.undo (thisCommand, cmdName);
}
@ -398,6 +408,13 @@ namespace control {
}
ExecResult
Command::undo (HandlingPattern::ID pattID)
{
return undo (HandlingPattern::get(pattID));
}
ExecResult
Command::execSync ()
{

View file

@ -139,6 +139,7 @@ namespace control {
ExecResult operator() () ;
ExecResult exec () ;
ExecResult undo () ;
@ -149,6 +150,9 @@ namespace control {
ExecResult exec (HandlingPattern const& execPattern);
ExecResult exec (HandlingPattern::ID);
ExecResult undo (HandlingPattern const& execPattern);
ExecResult undo (HandlingPattern::ID);
/** invoke using a default "synchronous" execution pattern */
ExecResult execSync ();
@ -204,6 +208,18 @@ namespace control {
return exec (getDefaultHandlingPattern());
}
inline ExecResult
Command::exec ()
{
return exec (getDefaultHandlingPattern());
}
inline ExecResult
Command::undo ()
{
return undo (getDefaultHandlingPattern());
}
template<typename...TYPES>
inline Command&

View file

@ -38,7 +38,6 @@ namespace proc {
namespace control {
namespace error = lumiera::error;
/** retrieve pre-configured pattern */
HandlingPattern const&
HandlingPattern::get (ID id)
{
@ -46,13 +45,12 @@ namespace control {
}
/** @param name to use in log and error messages
* @note does error handling, but delegates the actual
* execution to the protected (subclass) member */
/** @internal dispatch to the desired operation, with error handling */
ExecResult
HandlingPattern::invoke (CommandImpl& command, Symbol name) const
HandlingPattern::invoke (CommandImpl& command, string id, Action action) const
{
TRACE (proc_dbg, "invoking %s...", name.c());
const char* cmdName = cStr(id);
TRACE (proc_dbg, "invoking %s...", cmdName);
static _Fmt err_pre ("Error state detected, %s *NOT* invoked.");
static _Fmt err_post ("Error state after %s invocation.");
static _Fmt err_fatal ("Execution of %s raised unknown error.");
@ -63,7 +61,7 @@ namespace control {
return ExecResult (error::Logic (err_pre % command, errID_pre));
// execute or undo it...
perform (command);
(this->*action) (command);
Symbol errID = lumiera_error();
if (errID)
@ -76,21 +74,21 @@ namespace control {
catch (lumiera::Error& problem)
{
Symbol errID = lumiera_error();
WARN (command, "Invocation of %s failed: %s", name.c(), problem.what());
WARN (command, "Invocation of %s failed: %s", cmdName, problem.what());
TRACE (proc_dbg, "Error flag was: %s", errID.c());
return ExecResult (problem);
}
catch (std::exception& library_problem)
{
Symbol errID = lumiera_error();
WARN (command, "Invocation of %s failed: %s", name.c(), library_problem.what());
WARN (command, "Invocation of %s failed: %s", cmdName, library_problem.what());
TRACE (proc_dbg, "Error flag was: %s", errID.c());
return ExecResult (error::External (library_problem));
}
catch (...)
{
Symbol errID = lumiera_error();
ERROR (command, "Invocation of %s failed with unknown exception; error flag is: %s", name.c(), errID.c());
ERROR (command, "Invocation of %s failed with unknown exception; error flag is: %s", cmdName, errID.c());
throw error::Fatal (err_fatal % command, errID);
}
}
@ -100,8 +98,7 @@ namespace control {
/* ====== execution result state object ======= */
/** @note we just grab and retain the error message.
* @todo rather keep the exception object around. */
/** @note we just grab and retain the error message. */
ExecResult::ExecResult (lumiera::Error const& problem)
: log_(problem.what())
{ }

View file

@ -88,21 +88,17 @@ namespace control {
/**
* Operation Skeleton how to invoke or undo a command.
* Concrete implementations may be retrieved by ID;
* they range from just invoking the command operations
* straight forward to dispatching with the ProcDispatcher
* or running the command asynchronously in a background thread.
* A HandlingPattern first of all describes how to invoke the
* command operation, but for each pattern it is possible to
* get a special "undo pattern", which, on activation, will
* reverse the effect of the basic pattern.
* Interface: Operation Skeleton how to invoke or undo a command.
* Concrete implementations may be retrieved by ID; they range
* from just invoking the command operations straight forward
* to dispatching with the ProcDispatcher or running the command
* asynchronously in a background thread.
*/
class HandlingPattern
: public lib::BoolCheckable<HandlingPattern>
{
public:
virtual ~HandlingPattern() {} ///< this is an interface
enum ID
{ SYNC
@ -115,37 +111,50 @@ namespace control {
static ID defaultID() { return DUMMY; } ///////////TODO: should be ID::SYNC Ticket #211
/** retrieve the pre-configured pattern */
static HandlingPattern const& get (ID id);
virtual ~HandlingPattern() {}
/** main functionality: invoke a command, detect errors.
* @return ExecResult object, which might later be used to
* detect errors on execution */
ExecResult invoke (CommandImpl& command, Symbol name) const;
* @param string id of the command for error logging
* @return ExecResult object, which might later be used
* to detect errors on execution */
ExecResult exec (CommandImpl& command, string) const;
/** @return HandlingPattern describing how the UNDO operation is to be performed */
HandlingPattern const& howtoUNDO() const { return getUndoPatt(); }
/** likewise invoke the configured UNDO operation */
ExecResult undo (CommandImpl& command, string) const;
virtual bool isValid() const =0;
protected:
virtual HandlingPattern const& getUndoPatt() const =0;
virtual void perform (CommandImpl& command) const =0;
virtual void exec (CommandImpl& command) const =0;
virtual void undo (CommandImpl& command) const =0;
virtual void performExec (CommandImpl& command) const =0;
virtual void performUndo (CommandImpl& command) const =0;
private:
typedef void (HandlingPattern::*Action) (CommandImpl&) const;
ExecResult invoke (CommandImpl&, string id, Action) const;
};
inline ExecResult
HandlingPattern::exec (CommandImpl& command, string id) const
{
return this->invoke (command, id, &HandlingPattern::performExec);
}
inline ExecResult
HandlingPattern::undo (CommandImpl& command, string id) const
{
return this->invoke (command, id, &HandlingPattern::performUndo);
}
}} // namespace proc::control
#endif

View file

@ -68,7 +68,7 @@ namespace control {
bool isValid() const { return true; }
void
exec (CommandImpl& command) const
performExec (CommandImpl& command) const override
{
REQUIRE (command.canExec());
command.invokeCapture();
@ -76,42 +76,11 @@ namespace control {
}
void
undo (CommandImpl& command) const
performUndo (CommandImpl& command) const override
{
REQUIRE (command.canUndo());
command.invokeUndo();
}
/* == invoking operation or undo == */
void perform (CommandImpl& command) const { return exec(command); }
class UndoProxyPattern ///< standard UNDO implementation by reconfiguring a base Pattern
: public HandlingPattern
{
BasicHandlingPattern& basePatt_;
bool isValid() const { return basePatt_.isValid(); }
void exec (CommandImpl& command) const { return basePatt_.exec(command); }
void undo (CommandImpl& command) const { return basePatt_.undo(command); }
void perform (CommandImpl& command) const { return undo(command); }
HandlingPattern const& getUndoPatt() const { return basePatt_; }
public:
UndoProxyPattern (BasicHandlingPattern& refPattern)
: basePatt_(refPattern)
{ }
};
HandlingPattern const& getUndoPatt() const { return standardUndoPattern_; }
UndoProxyPattern standardUndoPattern_;
friend class UndoProxyPattern;
public:
BasicHandlingPattern()
: standardUndoPattern_(*this)
{ }
};
@ -119,25 +88,25 @@ namespace control {
/**
* Handling Pattern: invoke blocking, translate exceptions into an error state
* @todo describe this pattern in more detail....
* @todo unimplemented...
* @todo unimplemented... //////////////////////////////////////////////////////////TICKET #210
*/
class InvokeSyncNoThrow
: public BasicHandlingPattern
{
void
exec (CommandImpl& command) const
performExec (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually invoke a command, according to this pattern");
}
void
undo (CommandImpl& command) const
performUndo (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually undo the effect of a command, according to this pattern");
}
bool
isValid() const
isValid() const override
{
UNIMPLEMENTED ("is this pattern currently able to handle commands?");
}
@ -149,25 +118,25 @@ namespace control {
* Handling Pattern: invoke blocking, propagating any exceptions immediately
* @todo is throwing here helpful, and how to integrate it into ExecResult...?
* @todo describe this pattern in more detail....
* @todo unimplemented...
* @todo unimplemented... //////////////////////////////////////////////////////////TICKET #210
*/
class InvokeSyncThrow
: public BasicHandlingPattern
{
void
exec (CommandImpl& command) const
performExec (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually invoke a command, according to this pattern");
}
void
undo (CommandImpl& command) const
performUndo (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually undo the effect of a command, according to this pattern");
}
bool
isValid() const
isValid() const override
{
UNIMPLEMENTED ("is this pattern currently able to handle commands?");
}
@ -179,25 +148,25 @@ namespace control {
* Handling Pattern: just schedule command to be invoked asynchronously
* @todo clarify what "async" means and if we need it.....
* @todo describe this pattern in more detail....
* @todo unimplemented...
* @todo unimplemented... //////////////////////////////////////////////////////////TICKET #210
*/
class InvokeAsync
: public BasicHandlingPattern
{
void
exec (CommandImpl& command) const
performExec (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually invoke a command, according to this pattern");
}
void
undo (CommandImpl& command) const
performUndo (CommandImpl& command) const override
{
UNIMPLEMENTED ("actually undo the effect of a command, according to this pattern");
}
bool
isValid() const
isValid() const override
{
UNIMPLEMENTED ("is this pattern currently able to handle commands?");
}

View file

@ -99,7 +99,7 @@ return: 0
END
PLANNED "Command usage aspects II" CommandUse2_test <<END
TEST "Command usage aspects II" CommandUse2_test <<END
return: 0
END

View file

@ -138,30 +138,29 @@ namespace test {
// prepare for command invocation on implementation level....
HandlingPattern const& testExec = HandlingPattern::get(TEST_HANDLING_PATTERN);
HandlingPattern const& testUndo = testExec.howtoUNDO();
command1::check_ = 0;
bindRandArgument (*orig);
CHECK ( orig->canExec());
CHECK (!orig->canUndo());
testExec.invoke (*orig, "Execute original"); // EXEC 1
testExec.exec (*orig, "Execute original"); // EXEC 1
long state_after_exec1 = command1::check_;
CHECK (command1::check_ > 0);
CHECK (orig->canUndo());
CHECK (orig != copy);
CHECK (!copy->canUndo());
testExec.invoke (*copy, "Execute clone"); // EXEC 2
testExec.exec (*copy, "Execute clone"); // EXEC 2
CHECK (command1::check_ != state_after_exec1);
CHECK (copy->canUndo());
CHECK (copy != orig);
// invoke UNDO on the clone
testUndo.invoke (*copy, "Undo clone"); // UNDO 2
testExec.undo (*copy, "Undo clone"); // UNDO 2
CHECK (command1::check_ == state_after_exec1);
// invoke UNDO on original
testUndo.invoke (*orig, "Undo original"); // UNDO 1
testExec.undo (*orig, "Undo original"); // UNDO 1
CHECK (command1::check_ ==0);
CHECK (copy != orig);

View file

@ -27,6 +27,7 @@
#include "proc/control/command-def.hpp"
#include "proc/control/handling-pattern.hpp"
#include "lib/format-string.hpp"
#include "lib/format-obj.hpp"
#include "lib/util.hpp"
#include "proc/control/test-dummy-commands.hpp"
@ -64,7 +65,7 @@ namespace test {
protocolled (TY val2check)
{
return contains ( command2::check_.str()
, lexical_cast<string> (val2check)
, util::toString (val2check)
);
}
@ -72,9 +73,9 @@ namespace test {
/***********************************************************************//**
* @test command usage aspects II: patterns of invoking commands.
* @test command usage aspects II: patterns of command invocation.
*
* @todo this test is still on hold, as the non-trivial patterns aren't implemented as of 10/09
* @todo this test is still on hold, as the non-trivial patterns aren't implemented as of 10/09 ////////////////TICKET #211
*
* @see Command
* @see command-basic-test.cpp (simple usage example)
@ -109,15 +110,16 @@ namespace test {
.operation (command2::operate)
.captureUndo (command2::capture)
.undoOperation (command2::undoIt)
.bind (randFun, ref(blowUp_));
.bind (randFun, &blowUp_);
//note : blowUp_ is bound via reference_wrapper,
//note : blowUp_ is bound via pointer,
// thus we can provoke an exception at will.
blowUp_ = false;
check_defaultHandlingPattern();
check_ThrowOnError();
// check_ThrowOnError(); //////////////////////////////////////////////////////////////////////TICKET #211
// check_DispatcherInvocation() // yet to be written as of 1/2016
Command::remove ("test.command2");
@ -135,29 +137,28 @@ namespace test {
CHECK (!protocolled("invoked"));
bool res = com();
bool success = com();
CHECK (res);
CHECK (success);
CHECK (protocolled("invoked"));
CHECK (protocolled(randVal_));
res = com.undo();
CHECK (res); // UNDO invoked successfully
success = com.undo();
CHECK (success); // UNDO invoked successfully
CHECK (!protocolled(randVal_));
CHECK (protocolled("UNDO"));
blowUp_ = true;
string current = command2::check_.str();
res = com();
CHECK (!res); // not executed successfully (exception thrown)
success = com();
CHECK (not success); // NOT executed successfully (exception thrown and caught)
CHECK (command2::check_.str() == current);
CHECK (LUMIERA_ERROR_EXTERNAL == lumiera_error());
CHECK (!lumiera_error_peek()); // already absorbed
res = com.undo();
CHECK (!res); // UNDO failed (exception thrown)
success = com.undo();
CHECK (not success); // UNDO failed (exception thrown and caught)
CHECK (command2::check_.str() == current);
CHECK (LUMIERA_ERROR_EXTERNAL == lumiera_error());
blowUp_ = false;
}

View file

@ -135,7 +135,7 @@ namespace test {
command1::check_ = 0;
HaPatt patt = HandlingPattern::get(TEST_PATTERN);
ExecResult res = patt.invoke(*com, TEST_CMD);
ExecResult res = patt.exec (*com, string(TEST_CMD));
CHECK (res);
CHECK (ARGU == command1::check_);
@ -153,8 +153,7 @@ namespace test {
CHECK (command1::check_ > 0);
HaPatt ePatt = HandlingPattern::get(TEST_PATTERN);
HaPatt uPatt = ePatt.howtoUNDO();
ExecResult res = uPatt.invoke(*com, TEST_CMD);
ExecResult res = ePatt.undo (*com, string(TEST_CMD));
CHECK (res);
CHECK (command1::check_ == 0);

View file

@ -97,25 +97,25 @@ namespace test {
typedef function<string()> FunS;
inline void
operate (FunS func, bool fail)
operate (FunS func, bool *fail)
{
if (fail) throw External("simulated exception");
if (fail and *fail) throw External("simulated exception");
check_ << func();
}
inline string
capture (FunS, bool)
capture (FunS, bool*)
{
return check_.str();
}
inline void
undoIt (FunS, bool fail, string previousProtocol)
undoIt (FunS, bool *fail, string previousProtocol)
{
if (fail) throw External("simulated exception in UNDO");
if (fail and *fail) throw External("simulated exception in UNDO");
check_.seekp(0);
check_.str("");
check_ << previousProtocol << "|UNDO|";
}