Hi Alexander Thanks for your review!
Most things are quite understandable. However, code snipsets are quite complex. On my first reading, I stopped trying to understand them after a second of third sample. May be it's only me who reads complex stuff in a such way but the fact that simple_state class accepts up to five parameters gives a hint to a user that there shall be not_so_simple_state class that accepts 6+ or is more complex in other ways.
Actually I never liked the name of the simple_state class template but I can't really think of a better one. The rationale is: You can do everything with state, "simple_state" should suggest that you can do less with it. It is difficult to pack the real difference in a short descriptive name. Suggestions welcome.
This appears at the beginning and I suspect some will stop reading the tutorial right at that point. I'd suggest using some smart intendation at least.
There's going to be an overhaul of the tutorial, including the indentation.
* What is your evaluation of the design? What features are supported better by alternative FSMs? What could be added (or removed) from the library?
I don't like the design. Primary goal of the library is to be close to UML and the secondary goal is performance. UML conformance is pushed so strongly that any attempt to improve performance makes design worse.
It would be interesting to hear how you came to that conclusion. The UML conformance has *nothing* to do with the current state of affairs regarding performance. I think it is possible to write a UML conformant FSM lib that has the often quoted "ruthless" efficiency. The main reason for the perceived "bad" performance are the scalability requirements. And believe me: There are applications that absolutely need that scalability.
For example, deferral<XXX> requires that event is allocated using new and pointed by intrusive_ptr. Ordinary events can be allocated differently. If performance was out of library scope, the library could copy _all_ events into heap-allocated buffer. From user point of view, all event kinds looked the same, then.
I don't understand. The requirement exists exactly because anything else would be less efficient (space, time, executable size). Users who don't care can heap-allocate all events anyway.
I believe that performance is not so important because even optimized, the library is too slow for FSM.
Too slow for Finite State Machines? I can't make any sense of this.
According to docs, process_event function has 10 steps and there is a search in a couple of steps. This means loops and branching.
Don't take this description too literally. Much of this searching could theoretically be eliminated. Some branches can be (and are) done at compile time.
I always thought that process_event in typical FSM is single jump followed by indirect function call.
Yes, that's the common way to implement simple FSMs. Problem is: You don't get very far that way when your FSMs become bigger.
There are other things I don't like. One example is that transit call in react is similar to "delete this;"
Please suggest an alternative.
The "Unstable state" and "Unstable state machine" sections are too among things I don't like. They exist because transition is a complex process which involves multiple destructions and constructions.
No. Any state machine can become unstable as soon as you have both of the following: 1. Hierarchical states 2. Failing actions (doesn't matter what: entry, exit, transition) This has *nothing* to do with the interface or the implementation of the proposed FSM library. If you were to use the library and you don't want your FSM to become unstable, then either: - Don't let actions fail - Don't use hierarchical states - Always use null_exception_translator You really do have a choice here!
Not only it affects performance
Yes, it does affect performance *slightly*, but what good is performance when your code isn't correct?
but it complicates recovering from exception.
The exact opposite is true. Please explain how you came to this conclusion.
* The library documentation contains few not yet solved issues (name, separating the library into two parts, exception handling). What is you opinion here?
I'm stongly against name FSM
As I said: I'm not against changing the name
because the library doesn't give enough guarantees common in FSM world. Because the library is not predicitable in real-time terms
I suggest you pinpoint the locations in the code that you think cause non-predictability. [snip]
* Are there performance bottlenecks? Does the library design fit requirements of real-time systems? How is it useable for embedded systems? Is the documentation clear on performance tradeoffs?
I don't think it's acceptable for real-time systems.
We seem to have different definitions of what real-time means. Real-time does *not* mean fast. Real-time means that you can specify an upper limit of how much time it will take to process an event. I don't think anything in the proposed FSM library prevents you from doing that.
* What is your evaluation of the potential usefulness of the library? Can you compare this FSM to other implementations?
It's definitely useful as a library for representing UML statechart but it covers only subset of FSM.
You mean it only covers the subset of FSMs that don't need ruthless efficiency? I could certainly agree with that. [snip]
I'm afraid not. Either it should be repositioned as Boost.Statecharts or redesigned for better real-time support.
See above. Regards, -- Andreas Huber When replying by private email, please remove the words spam and trap from the address shown in the header.