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