Skip to content

Part four#8

Open
oleksiipet wants to merge 7 commits into
hyperskill:masterfrom
oleksiipet:part-four
Open

Part four#8
oleksiipet wants to merge 7 commits into
hyperskill:masterfrom
oleksiipet:part-four

Conversation

@oleksiipet
Copy link
Copy Markdown

No description provided.

@swsms swsms requested review from aaaaaa2493 and swsms and removed request for aaaaaa2493 December 27, 2018 16:05
Comment thread src/blockchain/Blockchain.java Outdated
}

public synchronized Block tail() {
return blocks.get(blocks.size() - 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you want, you may rewrite this code using ReentrantReadWritelock to separate lock for write and read operations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Cheers. Very good idea.

Comment thread src/blockchain/Blockchain.java Outdated

private boolean isValid(Block newBlock) {
Block tailBlock = blocks.get(blocks.size() - 1);
if (!newBlock.getHash().startsWith(getPrefix()) || !newBlock.getHashPreviousBlock()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Objects.equals(...,...) is better to avoid potential NPE

Comment thread src/blockchain/Blockchain.java Outdated
for (int i = 1; i < blocks.size(); i++) {
Block prev = blocks.get(i - 1);
Block cur = blocks.get(i);
if (!cur.getHashPreviousBlock().equals(prev.getHash())) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think Objects.equals(...) is better here

}

private void adjustComplexity(Block newBlock) {
if (!withinAcceptable(newBlock)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is good idea to encapsulate the condition's logic in a separated method

Comment thread src/blockchain/data/Message.java Outdated

public class Message {

private String author;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You may mark these fields as final

Comment thread src/blockchain/Block.java
private String data;


public Block(Integer id, Long minerId, String hashPreviousBlock, String hash, Long timestamp,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the constructor takes a lot of parameters. You may try to use the Builder pattern or combine some parameters together in a single entity

Comment thread src/blockchain/Main.java Outdated
Blockchain<Message> blockchain = new Blockchain<>(persister,
new PlanMessageFormat());

Thread[] miners = new Thread[NUMBER_OF_MINERS];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Java 8+ way:

List<Thread> miners = Stream
        .generate(() -> new Thread(() -> System.out.println("hello")))
        .limit(100)
        .collect(Collectors.toList());
				
miners.forEach(Thread::start);
				
miners.forEach(Thread::interrupt);

Comment thread src/blockchain/Block.java
@@ -0,0 +1,57 @@
package blockchain;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code looks good and readable. Convenient separation into classes and methods.

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.

3 participants