Skip to content

Conversation

@oleksiipet
Copy link

No description provided.

@swsms swsms requested review from aaaaaa2493 and swsms and removed request for aaaaaa2493 December 27, 2018 16:05
}

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

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
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.


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

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

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

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

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


public class Message {

private String author;
Copy link

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,
Copy link

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

Blockchain<Message> blockchain = new Blockchain<>(persister,
new PlanMessageFormat());

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

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;
Copy link

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