-
Notifications
You must be signed in to change notification settings - Fork 6
Part four #8
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
base: master
Are you sure you want to change the base?
Part four #8
Conversation
src/blockchain/Blockchain.java
Outdated
| } | ||
|
|
||
| public synchronized Block tail() { | ||
| return blocks.get(blocks.size() - 1); |
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.
If you want, you may rewrite this code using ReentrantReadWritelock to separate lock for write and read operations.
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.
Cheers. Very good idea.
src/blockchain/Blockchain.java
Outdated
|
|
||
| private boolean isValid(Block newBlock) { | ||
| Block tailBlock = blocks.get(blocks.size() - 1); | ||
| if (!newBlock.getHash().startsWith(getPrefix()) || !newBlock.getHashPreviousBlock() |
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.
Objects.equals(...,...) is better to avoid potential NPE
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())) { |
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.
I think Objects.equals(...) is better here
| } | ||
|
|
||
| private void adjustComplexity(Block newBlock) { | ||
| if (!withinAcceptable(newBlock)) { |
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.
It is good idea to encapsulate the condition's logic in a separated method
src/blockchain/data/Message.java
Outdated
|
|
||
| public class Message { | ||
|
|
||
| private String author; |
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.
You may mark these fields as final
| private String data; | ||
|
|
||
|
|
||
| public Block(Integer id, Long minerId, String hashPreviousBlock, String hash, Long timestamp, |
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.
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
src/blockchain/Main.java
Outdated
| Blockchain<Message> blockchain = new Blockchain<>(persister, | ||
| new PlanMessageFormat()); | ||
|
|
||
| Thread[] miners = new Thread[NUMBER_OF_MINERS]; |
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.
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);
| @@ -0,0 +1,57 @@ | |||
| package blockchain; | |||
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.
The code looks good and readable. Convenient separation into classes and methods.
No description provided.