Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deep history #33

Closed
Voskuijlen opened this issue Jan 7, 2015 · 8 comments
Closed

Deep history #33

Voskuijlen opened this issue Jan 7, 2015 · 8 comments

Comments

@Voskuijlen
Copy link

Currently to my experience every child of a state with a deep historytype has to define a historytype, which is kind of strange because that statemachine will return to the last active state on every transition in that statemachine.

I looked at the code and guess that commenting/removing line 170 and 172 of the fsm.impl.StateImpl.java (if(getParentState().getHistoryType()!=HistoryType.NONE) {}) will solve this problem, because currently the state is only saved if a historytype is defined. Removing this, will to my idea, result in not having to define a historytype in every layer of the statemachine, being a better implementation of the deep history.

This comes because not only the direct parentstate, but maybe the parent of the parent of the parentstate might have the deep history defined.

I would like to know your opinion on this matter.

For example:
Giving these states:
@States({
@State(name="Ref17_A", historyType=HistoryType.DEEP),
@State(name="Ref17_A_A1", parent="Ref17_A", initialState=true),
@State(name="Ref17_A_A1_A1_1", parent="Ref17_A_A1", initialState=true),
@State(name="Ref17_A_A1_A1_2", parent="Ref17_A_A1"),
@State(name="Ref17_B"),
})

By transiting to Ref17_A_A1_A1_2, and after that going to Ref17_B, then returning to Ref17_A will only set the statemachine back to Ref17_A_A1_A1_1 instead of the supposed Ref17_A_A1_A1_2.

@hekailiang
Copy link
Owner

Hi Voskuijlen, Thanks for reporting this issue. I will look into this problem and come back to you later.

@Voskuijlen
Copy link
Author

Hi Hekailiang, Thank you for looking in to this problem. When do you think this might be solved?

@hekailiang
Copy link
Owner

Hi Voskuijlen, I think your thoughts is reasonable. However, my concern is about memory consumption. By doing that you recommended it will cause all the historical information being stored no matter necessary or not. Since this is not a functional problem I will keep this issue open, and try to provider better solution later when I have time. In the mean time if you have better solution I will be happy to take a look. I am sorry for the late reply.

@Voskuijlen
Copy link
Author

Hi Hekailiang, I don't know how the memory consumption will act further down the road. Perhaps adding this else-statement to the if-statement something like the following will be better then?:

if(getParentState().getHistoryType()!=HistoryType.NONE) {
   stateContext.getStateMachineData().write().lastActiveChildStateFor(getParentState().getStateId(), getStateId());
}
else {
   ImmutableState<T, S, E, C> parent = getParentState();
   while (parent != null) {
      if(parent.getHistoryType()==HistoryType.DEEP) {
         stateContext.getStateMachineData().write().lastActiveChildStateFor(parent.getStateId(), getStateId());
         parent = null;
      }
      else {
         parent = parent.getParentState();
      }
   }
}

This will look for the parent state as long as they exist and if historytype deep is found, the state will saved.

Although I am not sure if this also solves the following problem:
A submachine state should be semantically be the same as a composite state (according to http://www.uml-diagrams.org/state-machine-diagrams.html). This means that deep history should also apply to submachine states (or linked states in Squirrel). I don't know if the getParentState function returns the submachine state if linked?

@hekailiang
Copy link
Owner

Hi Voskuijlen, you changes looks fine for me. Can you submit a pull request and with test case provided for your changes.

The linked state backed by a state machine to process its event. Currently implementation(actually is not good) stores all the state machine instances within linked states, and will terminate backend state machine instance when linked state exit, which cause historical information of backend state machine lost. I guess nobody use this feature right now(otherwise it should be many issues) you can ignore about linked state.

@Voskuijlen
Copy link
Author

Hi Hekailiang, I think I did something wrong. I forked the project, and don't really know how to create a pull request from this.. any advise?

@hekailiang
Copy link
Owner

@Voskuijlen
Copy link
Author

Ok found it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants