[boost] [msm] Review

Subject: [boost] [msm] Review
From: Michael Caisse (boost_at_[hidden])
Date: 2009-12-14 15:51:50

Let me start with a thank you. Thank you Christophe for
taking the time to "boostify" and submit this library.
Thank you also for your quick responses to questions and
discussions through out the review.

I apologize that this is "late". I was unable to complete
the review over the weekend.

I am afraid that this review is somewhat disjointed. There
are two parts. The top section answers the standard
questions and provides some overview. The bottom section
provides some specific details about the documentation and
also discussions about specific features covered in each
document section. I found that reviewing in order of the
tutorial provided some structure to the review.

Some of the points I make have been brought out in other
emails/reviews ... but this is my review so I felt compelled
to write down what I found important.

Best regards -
Michael Caisse

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Executive Summary:

I personally find this library both useful and well
implemented for many of my needs. The documentation is
lacking and must be extended to include a library
reference. Additionally, I would like to see SimpleStates
implemented to support internal transitions as called
out in the standard. Other issues are detailed; however,
do not affect my qualified vote of "acceptance". My
conditional recommendation for acceptance (yes vote)
is based on resolving the documentation issues and lack of
SimpleState internal transitions.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

------------------------------------------------------
What is your evaluation of the design?
------------------------------------------------------

The design seems sound; however, it has a feel of being
in flux when reading the documentation. There are perhaps
too many methods to create row entries. I haven't thought
too much about this, but it seems that the functor (Row)
mechanism might be extended further to replace the various
"arow" syntax completely.

------------------------------------------------------
What is your evaluation of the implementation?
------------------------------------------------------

 From the point-of-view of a UML implementation, I think
this library does a good job of implementing the most used
features of the language.

I would like to see the addition of internal transitions
for SimpleStates before the acceptance of the library. Lack
of this feature makes it awkward at best to implement
constructs that are the UML state machine equivalent of
RAII. Entry and exit code for a state are often used to
do things like start/stop timers and other such resource
management. Not being able to handle internal transitions
(transitions that don't leave the state and therefore
do not cause an exit/entry cycle) make modeling this
concept impossible. Hacks can be applied to work around
the shortcoming ... but implementing the feature is the
right solution.

I find myself missing choice points. Using "anonymous
transitions" nearly allows the same semantic... but
not quite.

I am used to being able to define multiple triggers
for a transition. This is not a deal-killer but it would
be nice to be able to somehow do this. Perhaps I could
by having event 'a' and event 'b' derive from a common
base and then using the 'base' as the event. I'm
not sure. I also miss not having a "any" or "*" trigger
definition. In essence, if no transition match is found for
the active trigger ... take this transition.

I have not spent enough time with the source code
to provide an evaluation of the source.

------------------------------------------------------
What is your evaluation of the documentation?
------------------------------------------------------

The provided documentation is basically a tutorial. I found that
a library reference guide was lacking. Though the tutorial was
excellent, not having a library reference made using/reviewing
the library difficult. I'm sure that there are many features
and exploits that I didn't discover because the only route
left to the reviewer was hope an example was in the tutorial,
used in the example source code or by rummaging through the
library source.

Please make sure to add a section describing the run-to-completion
semantics as well as the ordering of evaluation/execution
when a trigger if fired.

Additionally, an example of converting a runtime int to
a type is in order. Most people doing statemachine work are
acting on some trigger that now needs to be translated to
a data type. There are many ways to slice this but none are
listed. When showing this to another programmer the response
I got was, "but I have ints or chars that I'm acting on. How
do I make that into a type?" External stimulus is not a type so
some hint on ways to do this will be nice for those getting
introduced the concept for the first time.

As an aside, I would be happy to review and offer feedback
with documentation modifications. I may be able to offer some
assistance with modifications also.

Additional documentation edits can be found at the end
of this review.

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

I find this library very useful. I have already begun
modifying my code generators that take ArgoUML and Poseidon
diagrams and generate code based on distributed machines
to use MSM as a backend. This will allow me to fully understand
the usefulness of the library to medium/large projects.

At this time I have started using MSM for a smaller
state machine project. I am finding the library useful.

------------------------------------------------------
Did you try to use the library? 
------------------------------------------------------

 - With what compiler? 
    gcc-4.3.3 and VC 8
    My experience with gcc has been fine. My experience with VC8 ran
    into problems with the eUML. I just switched to linux/gcc since
    my target almost always involves a gcc compiler.
    I don't have enough concrete experience with VC8 and MSM to
    provide a solid viewpoint ... but my soft view is that it
    might need some more help.

 - Did you have any problems?
    eUML (as expected) didn't work with VC 8

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

I spent about 25 hours reviewing this library. I did
not spend very much time with the source code but instead
reviewing the documentation and testing the concepts/implementation
as they stack up to the UML 2.0 standard. I also spent
time implementing several of my own machines with the MSM
library to determine how applicable and conforming the
library might be.

------------------------------------------------------
Are you knowledgeable about the problem domain?
------------------------------------------------------

Yes. I have been dealing with UML based statemachine notation
before it was part of UML. I have my own product that implements
ROOM/UML distributed hierarchical FSMs that are generated from
diagrams. I have implemented several UML compliant state
machine libraries in a variety of languages.

------------------------------------------------------
Additional Documentation Comments
Other MSM comments in order of the docs...
------------------------------------------------------

General:

Inconsistent use of eUML. Sometimes it is EUML (particularly at the
start of a sentence.) I believe it should always be eUML.

Sub-state machines and composite states are not *exactly* the same
in UML. It seems that it is implied or stated in a few places that
it is. I would just state up front that Msm does not support
composite states; however, it does support the more general
solution of sub-state machines.

----------------------------
Section "Why Msm v2?"

"UML designers are also used to write..."
should be
"UML designers are also used to writing..."

----------------------------
"Msm v1.x Features / Design Goals"

If you are taking votes about multithreading ... please make it
some non-required layer or policy or let the user's discretion
play out. I will be using Msm on targets without an OS.

"Msm v1.x Features / Founding Example"

It might make sense to utilize UML terminology when discussing
the parts of a transition:

    current state -> source state
    next state -> target state
    action method -> effect or behavior
    event -> trigger

So... maybe not the event to trigger change. This is an interesting
concept in Msm. Often an event is considered as having a
trigger and optional data payload. The Msm event is a struct
with possible data members. Event is fine I guess.

"The event being passed the" -> "The event being passed to the"

---------------------------
"A more advanced example"

"The UML specification isn't too clear about the difference
between composite states and sub-state machines..."

That statement isn't exactly true. The specification is clear;
however, Msm has implemented the more feature rich concept
which is a superset of composite state. If there isn't
a runtime (resource) penalty for this, then I don't see
a problem.

--------------------------
"Orthogonal States..."

While the implementation of deferred events is correct according
to UML I have found that the ROOM concept is extremely useful
and have implemented it in my own libraries. You may want
want to consider it as a feature to MSM. The defer mechanism
is the same (you can defer any event in any state); however,
you can later "recall" from the defer queue. This has the
benefit of allowing a "simple" construct for handling deferred
events. You can defer the events at a parent composite state.
Once you enter the sub state handling certain (potentially deferred)
events you can then do a recall from the queue. This is often
done in the state's entry code. This pattern reduces the work
involved by taking advantage of the hierarchical aspect of UML states.

--------------------------
"Adding an history"

should be "Adding a history"

I personally am happy with the UML definition of histories and
use them often in error handling patterns for example.

Part of the power of UML states is the fact that they are
hierarchical. Without a deep history concept in MSM I am required
to specify a history policy for each contained sub-state machine
(composite state). I do find this rather annoying though
it is workable.

I do like the AlwaysHistory policy since that is my normal
use case when using histories.

--------------------------
"Using 'flags'"

My initial reaction to flags is "composite states already
supply this information". I can see that flags can provide
multiple slices of meta state information and *might* be
useful. I am in general leery of abstractions that obfuscate
additional state information. From a diagram POV, that
state information is missing. In my world this typically
means that the granularity of machines is wrong (I use
distributed state machines) or the state hierarchy is not
quite right. I have the same negative feeling about
storing a lot of *state* information in variables that
are acted upon in guards and such. It hides important
information from the diagram and makes the machine harder
to review and understand.

This is not an argument to remove flags. It is just my
first impression of the feature. I will need to play some
more with it. I can see potential uses.

--------------------------
"Anonymous transitions"

In the UML standard the term "Completion Transition"
is used for this concept. I like this term because it is
descriptive of what is happening. I also like it it because
this is a UML compliant library and using the UML terminology
helps understanding the concepts more quickly.

--------------------------
"The many ways to enter a composite state"

Explicit exit is often a problem. I found that in two
of my machines I tried converting that I use this mechanism.
I used an exit point to work around the missing feature. This
works ... but wasn't my intent. That said... I think
what you have is proper for a submachine state. If
MSM actually implemented composite states, then the lack
of this notation would concern me. This might just be
me, but the purpose of a submachine is to wrap a composite
state into a reusable object. Entry and exit points
therefore represent the public interface. Explicit
entry and exit would be like accessing private data.

"Important note 3" - I think this is important behavior
for the proper use of submachine states.

--------------------------
"Advanced"

--------------------------
"Conflicting Transitions"

This information is important. I think expanding the section
to discuss the run-to-completion semantics and the order/execution
rules followed when a trigger is received is vitally important.

Example, trigger A is received. The following steps are
performed in this order: ....

--------------------------
"Contaning state machine (deprecated)"

This type of thing just confuses the situation. You have a
recommended practice and the library is just now in review.
I would leave this out or if you feel it is important... add
an appendix for those using pre-boost versions of the library.

--------------------------
"New Msm V2.0 Features"

--------------------------
"Functor rows"

I personally like this format for describing a row. I hate
to suggest yet one more mechanism... but having "row" utilize
a "none" would be nice. Then there would be two mechanisms
in the table -- 'Row' for functors and 'row' for other types.
Better yet would be figuring out how to combine those into one.

--------------------------
"eUML"

I played with eUML some and I find myself on the fence. First
off as I had related earlier, I think the ordering is not natural.
I do like Rob Stewart's suggestion of:

   DestState() = CurrentState() + cool_event()[guard()]/(action())

I find my fence riding odd because I have always thought a DSEL
like this would be the cat's meow. I think it might be the
copious use of BOOST_TYPEOF is just hard on my eyes and getting used
to the ordering of build_state. All of those instantiations of objects
with the ().... it just looks messy to me.

I love the idea and I think it is nearly there. It just 'looks'
complicated when I quickly view the source.

In the end I didn't have enough time to finish reviewing this
portion of the library. My gut says that it might be almost ready
for prime time. It might just need some more help with the sugar
coating.

--------------------------
"Executable size"

A lot of my state machine targets have 256k flash (or less) and
minimal RAM. Size is a big concern to me. I'm also unsure about
these claims. Not that the claims are wrong ... but my own testing
has yet to convince me that I will be able to use MSM for those targets
yet. I always use -Os but I am still experimenting with the impact of
size with the various front-ends and row options.

eUML seems to be a non-starter for me and these platforms.

-- 
----------------------------------
Michael Caisse
Object Modeling Designs
www.objectmodelingdesigns.com