-
-
Notifications
You must be signed in to change notification settings - Fork 467
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
Conversation
@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. |
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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}. |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
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
No description provided.