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

106 hello thread exercise demo #109

Closed
wants to merge 9 commits into from
Closed

106 hello thread exercise demo #109

wants to merge 9 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 7, 2021

No description provided.

@tboychuk
Copy link
Contributor

@tkuzub could you please create a branch in this repository, instead of your fork. I would like to be able to checkout your branch and do the exercise, maybe even push some changes. So it will be more convenient if you use this repository instead of the fork.

Copy link
Contributor

@tboychuk tboychuk left a comment

Choose a reason for hiding this comment

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

@tkuzub Overall looks good. Two key things to work on:

  • Javadoc should be very clear
  • method names should state the action you are going to perform, which means that should start from verbs

Please take a look at my comments, and let's do another iteration.

* @param runnable the code you want to run in new thread
* @return a new thread
*/
public static Thread runningThread(Runnable runnable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub let's use verbs in method names. E.g. createThread(Runnable runnable)


public class HelloThreads {
/**
* Receives a {@link Runnable} parameter, and returns a {@link Thread}.
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub Javadoc should provide more details. In this case, it should say

Receives a {@link Runnable} parameter that holds the logic and creates a {@link Thread} instance based on it. 

* Receive a {@link Thread} parameter and start it
* @param thread the code you want to start thread
*/
public static void runningThreadViaStart(Thread thread) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub this one should be called startThread

* @param thread the code you want start thread and return tread name
* @return the name thread
*/
public static String runningThreadGetNameThread(Thread thread) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub this one should be called getThreadName

* @param thread the code you want start thread and return state
* @return the thread state
*/
public static Thread.State runningThreadGetStateThread(Thread thread) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub please use static import for class State

* @param runnable the code you want to run in new thread and start
* @return this thread
*/
public static Thread getSomeLogicRunningTreadAndReturnThisThread(Runnable runnable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub for instance this one can be called runInNewThread

@@ -25,6 +25,8 @@
<module>6-0-test-driven-development</module>
<module>java-fundamentals-util</module>
<module>lesson-demo</module>
<module>7-0-java-concurrency</module>
<module>7-0-java-concurrency/7-0-0-hello-threads</module>
Copy link
Contributor

Choose a reason for hiding this comment

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

@tkuzub as you can see, this pom file should hold only high-level modules. If you want to add a nested module, 7-0-0-hello-threads, you should specify it in 7-0-java-concurrency/pom.xml

@tboychuk tboychuk assigned ghost Oct 15, 2021
@tboychuk tboychuk linked an issue Oct 15, 2021 that may be closed by this pull request
@ghost ghost closed this Oct 15, 2021
This pull request was closed.
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

Successfully merging this pull request may close these issues.

Create new hello-threads exercise
2 participants