Re: [boost] [review] FSM Second Call for Reviews

From: Chris Knight (cknite_at_[hidden])
Date: 2008-08-29 18:05:08

On Monday 18 August 2008 7:21:51 pm Martin Vuille wrote:

> The formal review for Andrey Semashev's Finite State Machines (FSM)
> library has been running for a week now and continues until the 20th.

> What is your evaluation of the design?
I completed my FSM review by attempting to convert an existing implementation 
of the FIX transport protocol 
(http://www.fixprotocol.org/specifications/fixt1.1spec) that currently uses an 
in-house event driven state machine and swap it for the FSM's framework. 
Unfortunately, I was unable to agreeably implement such a system and I will 
now enumerate over the issues I came upon. 

The major architectural problem I see is the factoring of the transition 
handlers (which is usually defined in the transition table) into the state 
classes. This peculiar arrangement necessarily results in an overall verbose 
code structure. 

By placing the on_process() function in the state event handlers, FSM users 
are forced to define an is_applicable<> specialization and supply a static 
transit() function that defines the pattern matching rules the state machine 
applies to incoming events.  Factoring the transition handler into the various 
states results in the dispersion of the very transition table that defines 
the identity of a finite state machine. This was more than likely done in an 
attempt to optimize away the linked list pattern match (dispatch) found in 
the more common approach. Abstractly thinking however, the run-time switch 
based upon the current state of the fsm is going to occurr whether it be 
found in the fsm driver; whether it be found in the linked-list dispatch of 
CTM's fsm example or whether it be split amongst the various states. 

I do note my agreement that transition handler concept should be allowed to 
return values on invalid events rather than be forced to throw exceptions 
since in many cases an invalid event for a particular state is an error only 
when viewed at the level of the state machine's "protocol" and often times 
occurs just as often as the situation in which there is a valid match. 

This is somewhat provided with the switch_to<> mechanism however this seems to 
exist outside the realm of the well-defined state machine's transition rules. 
This approach forces error handling into what would best be 
described as the lexer (state_machine::process()'s callee) and thus outside 
the domain of the state machine. A better approach would be to allow dynamic 
posting of events from within transition handlers. These "error events" are 
thus included in the definition of the state machine's transtition table.

Furthermore, while CTM's example fsm does not allow transition handlers to 
prevent the transition from "matching", it mirrors my internal 
implementation in most other regards as it too is implemented as 
fold(transitions_for_current_state, default_event_dispatcher, 
event_dispatcher<_2, _1>>).

The last note I would make regards the BOOST_FSM_MUST_HANDLE_ALL_EVENTS macro 
one uses to enforce compile-time state machine consistency. This is rather a 
suprise as my expectation would be for this to be an intrinsic function of an 
fsm. 

> Do you think the library should be accepted as a Boost library?
I'd like to take this moment to thank Andrey for his submission as the 
submitted library's documentation was very complete and it is obvious how much 
time and effort was involved. 

While I believe its purpose is certainly a valid one and found the 
implementation to be a novel approach that made me think in some interesting 
ways (like switching my fsm's implementation to allow for additional dispatch 
algorithms such as a hash lookup or a jump table (packed or unpacked 
vectors)).

However, with the library considered as a whole, I respectfully submit my vote 
as No. 

- Chris Knight