Skip to content

Reactor #74 implemented - #242

Merged
npathai merged 18 commits into
masterfrom
Reactor
Sep 16, 2015
Merged

Reactor #74 implemented#242
npathai merged 18 commits into
masterfrom
Reactor

Conversation

@npathai

@npathai npathai commented Sep 6, 2015

Copy link
Copy Markdown
Contributor

I have implemented #74 using Java NIO framework. Currently the Reactor only supports server mode.

Tasks:

  • Implement design pattern and tests
  • Documentation
  • Google Style Guide (done partially)
  • Web site
  • Class diagram

@npathai

npathai commented Sep 6, 2015

Copy link
Copy Markdown
Contributor Author

JDP team please review.

@iluwatar

iluwatar commented Sep 8, 2015

Copy link
Copy Markdown
Owner

@npathai In general it looks great. I will review in more detail at earliest

  • 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?

@npathai

npathai commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

@iluwatar Thanks. No problem. Take your time.

iluwatar
iluwatar reviewed Sep 11, 2015
View reviewed changes
Comment thread reactor/src/main/java/com/iluwatar/reactor/app/App.java Outdated

@iluwatar iluwatar Sep 11, 2015

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to read about the participants of the pattern here. Handles, synchronous event demultiplexer, initiation dispatcher and event handlers - what class in the example implements what? I saw you have good comments on the actual classes, maybe some of that description can be put here also.

@npathai

npathai commented Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

@iluwatar thts immature of me nt to take care of final.. I relied on eclipse behavior.. By somehow I seem to have changed d settings... Will take care of that 2moro..

@iluwatar

iluwatar commented Sep 11, 2015

Copy link
Copy Markdown
Owner

I'm sorry for the late review 😞 You should now catch up the master branch, this is no longer up to date.

@iluwatar

iluwatar commented Sep 11, 2015

Copy link
Copy Markdown
Owner

I'm done reviewing for now.

@npathai

npathai commented Sep 12, 2015

Copy link
Copy Markdown
Contributor Author

All done. I have left comments on your points. Awaiting further comments :)

@iluwatar

iluwatar commented Sep 12, 2015

Copy link
Copy Markdown
Owner

It's ok now. We only need the class diagram and index.md.

@npathai

npathai commented Sep 14, 2015

Copy link
Copy Markdown
Contributor Author

@iluwatar Done. Added class diagram and index.md. Please review.

@iluwatar

iluwatar commented Sep 15, 2015

Copy link
Copy Markdown
Owner

Looks fine @npathai Feel free to merge!

@npathai
Work on #74, removed author name from all classes. [ci skip]. Author …
dbc2aca 
…names were added due to default eclipse configuration.
npathai added a commit that referenced this pull request Sep 16, 2015
@npathai
Merge pull request #242 from iluwatar/Reactor
e5f1ff1 
Reactor #74 implemented using Java NIO framework.
@npathai
npathai merged commit e5f1ff1 into master Sep 16, 2015
@npathai
npathai deleted the Reactor branch September 16, 2015 06:16
@npathai npathai mentioned this pull request Sep 16, 2015
Closed
pratigya0 pushed a commit to pratigya0/java-design-patterns that referenced this pull request Aug 3, 2023
@atai Атаи Рустам Бахтиярович
Fix links and add question about branch strategies (iluwatar#242)
ea6481e 
Co-authored-by: Атаи Рустам Бахтиярович <ratai@borlas.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status: under review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@npathai @iluwatar