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

Finite state machine #222

Conversation

stephen-lazarionok
Copy link

No description provided.

@npathai
Copy link
Contributor

npathai commented Aug 20, 2015

I will pick it up this weekend. As agreed in #221 I request you to format the code according to Google Style guide.

@markusmo3
Copy link
Contributor

Your diagram seems non-standard. Please use the method mentioned in the readme

For creating/editing UML diagrams you need ObjectAid UML Explorer for Eclipse.

It doesnt look bad but we want unity here. If you dont like the current way the diagrams are displayed then you are welcome to suggest other tools to create UML diagrams from source here #190

@stephen-lazarionok
Copy link
Author

@noobxgockel I do not have Eclipse installed... Could anybody help me and create a diagram from the classes I have?

@npathai I will format the code according to Google Style guide and will check in my changes.

@iluwatar
Copy link
Owner

  • add finite-state-machine as a module to the super pom.xml
  • tests are missing
  • diagram is non-standard (we can add that since you don't have Eclipse)
  • index.md missing

@iluwatar
Copy link
Owner

Looking at the paper and comparing it to the example I fail to see similar class structure. In the paper Context and States implement the same interface but in the example this seems to be not so.

I think Context should have references to the States and be responsible for State transitions. In the example there is FiniteStateMachine containing Context and method for State transitions.

@geniusgeek
Copy link

I read the paper and it was very interesting especially the state data
model, the Context , more importantly, the event abstraction transition and
Automation interface, but i still feel that we could do better. The problem
with their implementation is that the states have to be know in advanced,
and users may not find it easy adding states. Also i feel that the events
should be implemented as commands, so as to avoid declaring no-ops
methods(or methods we don't need in each state). The concept of data
abstraction(using data model) is fine with memory(no memory consideration
as to excess storage of state data) but what if data changes in each state
or if each state need explicit data or to return to previous state and
remember previous data{ for this i feel that the data model should be
implemented using a memento pattern}. With these few issues in mind, i feel
we can write a great implementation of the pattern.

On Sun, Aug 23, 2015 at 12:47 PM, Ilkka Seppälä notifications@github.com
wrote:

Looking at the paper
http://www.csee.wvu.edu/~ammar/rts/adv%20rts/statecharts%20patterns%20papers%20and%20%20examples/paper%20on%20state%20pattern%20B31-full.pdf
http://www.csee.wvu.edu/%7Eammar/rts/adv%20rts/statecharts%20patterns%20papers%20and%20%20examples/paper%20on%20state%20pattern%20B31-full.pdf
and comparing it to the example I fail to see similar class structure. In
the paper Context and States implement the same interface but in the
example this seems to be not so.

I think Context should have references to the states and be responsible
for State transitions. In the example there is FiniteStateMachine
containing Context and method for State transitions.


Reply to this email directly or view it on GitHub
#222 (comment)
.

@geniusgeek
Copy link

please i need comment based on my suggestion above. Also is anyone still
working on this pattern. or can i take it up?

On Tue, Sep 1, 2015 at 5:07 PM, Ekpe Samuel geniusgeek2014@gmail.com
wrote:

I read the paper and it was very interesting especially the state data
model, the Context , more importantly, the event abstraction transition and
Automation interface, but i still feel that we could do better. The problem
with their implementation is that the states have to be know in advanced,
and users may not find it easy adding states. Also i feel that the events
should be implemented as commands, so as to avoid declaring no-ops
methods(or methods we don't need in each state). The concept of data
abstraction(using data model) is fine with memory(no memory consideration
as to excess storage of state data) but what if data changes in each state
or if each state need explicit data or to return to previous state and
remember previous data{ for this i feel that the data model should be
implemented using a memento pattern}. With these few issues in mind, i feel
we can write a great implementation of the pattern.

On Sun, Aug 23, 2015 at 12:47 PM, Ilkka Seppälä notifications@github.com
wrote:

Looking at the paper
http://www.csee.wvu.edu/~ammar/rts/adv%20rts/statecharts%20patterns%20papers%20and%20%20examples/paper%20on%20state%20pattern%20B31-full.pdf
http://www.csee.wvu.edu/%7Eammar/rts/adv%20rts/statecharts%20patterns%20papers%20and%20%20examples/paper%20on%20state%20pattern%20B31-full.pdf
and comparing it to the example I fail to see similar class structure. In
the paper Context and States implement the same interface but in the
example this seems to be not so.

I think Context should have references to the states and be responsible
for State transitions. In the example there is FiniteStateMachine
containing Context and method for State transitions.


Reply to this email directly or view it on GitHub
#222 (comment)
.

@iluwatar
Copy link
Owner

I've provided my comments to @stephen-lazarionok The current review status is as follows.

  • Put under review badge to the pull request
  • Does the example code implement the pattern correctly?
  • Does the example code have enough test coverage?
  • Is the example code commented well enough?
  • Is the example code following JavaDoc conventions?
  • Are the project coding conventions being followed?
  • Is the class diagram generated correctly?
  • Is the index.md implemented correctly so the pattern will show correctly on the web site?

If there is no activity anymore on this PR I will close it this weekend and @geniusgeek can take it up.

@npathai
Copy link
Contributor

npathai commented Sep 12, 2015

@iluwatar see comment of Stephen on PR of active record pattern.. He seems to be on a vacation.

@iluwatar
Copy link
Owner

Thanks @npathai Ok, we can then wait a few more days. Maybe we can establish some sort of guideline from this, how long the PR's are kept open.

@npathai
Copy link
Contributor

npathai commented Sep 13, 2015

@iluwatar yes we can raise this point on chat room .

@iluwatar
Copy link
Owner

Due to no activity for 30 days I'm closing this PR.

@iluwatar iluwatar closed this Sep 21, 2015
@geniusgeek
Copy link

@iluwatar please i will start an activity on this as stated i am taking this up and will be active in in two days time.

@iluwatar
Copy link
Owner

Just comment on the issue #203 that you are working on it and when you're ready, create a pull request. This one is closed.

@geniusgeek
Copy link

ok

On Tue, Sep 22, 2015 at 7:56 PM, Ilkka Seppälä notifications@github.com
wrote:

Just comment on the issue #203
#203 that you
are working on it and when you're ready, create a pull request. This one is
closed.


Reply to this email directly or view it on GitHub
#222 (comment)
.

pratigya0 pushed a commit to pratigya0/java-design-patterns that referenced this pull request Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants