Skip to content

Conversation

@oleksiipet
Copy link

No description provided.

@swsms swsms self-requested a review January 9, 2019 12:56
Copy link

@swsms swsms left a comment

Choose a reason for hiding this comment

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

A good implementation with a large number of Java features. You can also try using var from newer versions of the language.

private List<T> data = List.of();


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

Choose a reason for hiding this comment

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

Personally, I don’t really like constructors with a lot of arguments (3-4+) of the same (String, String) or almost the same types (Integer, Long). But you use builder below which can be added inside this class to hide the constructor.

import java.io.Serializable;
import java.util.List;

public class Block<T extends SignedData & Serializable> implements Serializable {
Copy link

Choose a reason for hiding this comment

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

Good abstraction

readLock.lock();
return Stream.iterate("0", x -> "0")
.limit(prefixLength)
.reduce("", (x, y) -> x + y);
Copy link

Choose a reason for hiding this comment

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

There is a reduce-like function joining for strings. You may try to use it.

Block<T> prev = blocks.get(i - 1);
Block<T> cur = blocks.get(i);
Stream<T> concatData = Stream.concat(
Objects.requireNonNullElse(prev.getData(), new ArrayList<T>()).stream(),
Copy link

Choose a reason for hiding this comment

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

👍

do {
prefix = blockchain.getPrefix();
magicNumber = random.nextInt();
hash = StringUtil.applySha256(previousHash + magicNumber);
Copy link

Choose a reason for hiding this comment

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

I don't understand why "Message" is not a part of hash?

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.

4 participants