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

From: Thomas Klimpel (Thomas.Klimpel_at_[hidden])
Date: 2008-08-29 06:19:00

> What is your evaluation of the design?

When I look at finite state machine in Wikipedia, Fig. 1 shows me an "Example  of a Finite State Machine". The blue words "state", "transition", "transition condition", "entry action" in this picture probably represent some of the important notions in this domain. How does the FSM library relate to these concepts?

The "state" concept is modeled as a class that has to publicly derive from fsm::state<Classname,  StateList>. More on this later.

The "transition" concept is modeled as the class fsm::transition<InitialState, Event, FinalState>. User defined transition actions can derive from this class and overide the "transit" method.

The "transition condition" is replaced with the "event" concept. I guess this is OK, since the Wikipedia article also mentions event driven finite state machines. The "event" concept is modeled as an arbitrary class. There doesn't seem to be an EventList (analogous to the StateList) that lists all allowed events, so it's probably possible to send an arbitrary object to the state machine, which will then react somehow to that (unexpected?) event. In case a reaction to the "any_event" class is defined, the single value constructor (without the explicit keyword) of this class will probably convert this event to the "any_event".

I haven't investigated how the different types of "actions" are modeled by the library, or whether they are modeled at all.

The most important design decision seems to be let the state machine have a member of a class that derives from all states in StateList. The switching between the states now changes which of these "base classes" is "in charge" of handling events.

I see two reasons for this design choice. The first is performance, which is one of the main goals of the library, the second is that the library models a "state machine" instead of a "finite state machine". This is OK, but I would consequently prefer the name "state_machine" instead of "fsm" for the library.

The modeling of the "transition" concept could and should probably be improved. The transition table where I can specify for given "intial states" and "events" the resulting "final states" and "actions" (modeled as functions that take a const reference to the corresponding event) to perform appears superior to me. There might be other good ways to model this concept, but pointing out a single superior alternative should be sufficient to prove that the current design can be improved here.

The modeling of the events as arbitrary classes makes sense to me. I haven't investigated in detail the amount of support functionality related to the event concept. A predefined event class template that takes enums as template parameter, or some way to restrict (at compile time) the possible events handled by a state machine come to my mind. However, these are probably just minor details.

So in conclusion, most important design decisions are probably OK, but the design as a whole is not finished and polished enough.

> What is your evaluation of the implementation?

I did take a look at the implementation and tried to understand it. It may be OK, but it uses tools that I'm not sufficiently familiar with. An example of this is boost-preprocessor. The inh*.hpp files in boost/fsm/detail are all missing include guards. However, on closer inspection it turns out that this is intended, and that they are included multiple times (by some boost-preprocessor mechanism). But I ask myselfs whether they should not use a different extension than .hpp, since they are not really header files.

The general conclusion is that my current knowledge is sufficient to understand the implementation of

http://www.boostpro.com/mplbook/examples/player.cpp
http://www.boostpro.com/mplbook/examples/player2.cpp

but that I have a hard time understanding the implementation of boost/fsm/state_machine.hpp

> What is your evaluation of the documentation?

The documentation is fine.

> What is your evaluation of the potential usefulness of the library? 

I don't know.

> Did you try to use the library?

No.

> With what compiler? Did you have any problems? 

See above.

> How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? 

A quick reading.

> Are you knowledgeable about the problem domain? 

No.

> Do you think the library should be accepted as a Boost library?

No. However, this opinion is not really related to this review, but to the general reaction on the boost-mailing list about the potential usefulness of the library. There was nobody explicitly saying that he has an urgent need for this library.

Why did I submit this review then, when I didn't really have an interest in this library? I was following the discussions about the library, and tried to understand how this library differs from the other proposed approaches. So I finally ended up having thought so much about these issues that it made sense for me to submit a review.