Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

106 hello thread exercise demo #114

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

Closed
ghost wants to merge 12 commits into main from 106-hello-thread-exercise-demo
Closed

Conversation

Copy link

@ghost ghost commented Oct 15, 2021

No description provided.

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 please take a look at my comments. Thank you!

pom.xml Outdated
@@ -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>
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub

  • please move this line up (right after the 6th module)
  • please remove the next line (you should follow the same approach we have), in other words, this pom file declares only high-level modules. And 7-0-0-hello-threads should be declared as a submodule of 7-0-java-concurrency inside its pom file.

<version>1.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>
<artifactId>7-0-0-hello-theads</artifactId>
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub there is a mistake here: it should be 7-0-0-hello-threads not 7-0-0-hello-theads


import static java.lang.Thread.*;

public class HelloThreads {
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub a class should have a javadoc as well. For instance

/**
 * {@link HelloThreads} provide a simple API for working with Java threads. A {@link Thread} class represents a key
 * concept in Java Concurrency. So this class is meant to teach you the basics like creating a thread or checking its
 * state.
 * <p>
 * todo: implement each method according to javadoc and verify your impl by running {@link HelloThreadsTest}
 *
 * @author Taras Kuzub
 */

* @return the list of running threads
*/
@SneakyThrows
public static List<Thread> runInMultipleThreads(Runnable runnable) {
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub let's better add another parameter to specify the number of threads.

runningThread.start();
runningThread.join();

verifyContentQueue();
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub I don't like this approach with a shared queue. I guess it's better that you create a new instance of a queue in each test if you need and, then use it, and then verify what you want to verify.

thread.join();
assertEquals("name", threadName);

verifyContentQueue();
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

It should not assert the queue content, since this method is only created to the thread name.

thread.join();
assertSame(state, Thread.State.RUNNABLE);

verifyContentQueue();
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

It should not assert the queue content.


verifyContentQueue();
}

Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub every test should follow the patter

  • build – where you prepare everything you need
  • operate - where you perform the action that you want to test
  • assert - where you verify the results and check if they are correct
    Another similar pattern is given, when, then.

For the assertions we use AsserJ. So for example the following test should look like this:

 @SneakyThrows
 @Test
 @Order(3)
 void getThreadState() {
 var thread = new Thread(waitASecond());
 thread.start();
 
 var state = HelloThreads.getThreadState(thread);
 assertThat(state).isEqualTo(RUNNABLE);
 }

@@ -0,0 +1,84 @@
import com.bobocode.util.ExerciseNotCompletedException;
Copy link
Contributor

@tboychuk tboychuk Nov 5, 2021

Choose a reason for hiding this comment

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

@tkuzub please move all the code to package com.bobocode.concurrency

@ghost ghost self-assigned this Nov 18, 2021
@ghost ghost added the completed_solution All exercises completed label Dec 18, 2021
Copy link
Contributor

@tkuzub it looks like there is no progress on this task. I'm going to close this PR if you don't mind.

Copy link
Author

ghost commented Jun 27, 2022
edited by ghost
Loading

@tboychuk, Ok

@ghost ghost closed this Jun 27, 2022
This pull request was closed.
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
completed_solution All exercises completed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /