While lurking for injected bugs, I found that some parts of Nxt code makes my eyes bleed. With hope that I have enough programming skills (probably better, than my runglish 
), I'm starting this thread. Not for bounty, but for better quality (but donations welcome, as usual ).
First of all, thanks to IntelliJ IDEA code analysis, I see a lot of syncronized with potentially wrong usage.
Static variables like peers, blocks, accounts, users are not final. They are initialized at start, so it is not a big flaw right now, but who knows about future.
When I look more deeply in syncronizations, I see a lot of places, where author get content of such global collection and then sync on it, but not sync on collection itself. Example:
More to come...
), I'm starting this thread. Not for bounty, but for better quality (but donations welcome, as usual ).First of all, thanks to IntelliJ IDEA code analysis, I see a lot of syncronized with potentially wrong usage.
Static variables like peers, blocks, accounts, users are not final. They are initialized at start, so it is not a big flaw right now, but who knows about future.
When I look more deeply in syncronizations, I see a lot of places, where author get content of such global collection and then sync on it, but not sync on collection itself. Example:
Code:
Block.analyse() method:
Account generatorAccount = accounts.get(Account.getId(generatorPublicKey));
synchronized (generatorAccount) {
generatorAccount.setBalance(generatorAccount.balance + totalFee * 100L);
generatorAccount.setUnconfirmedBalance(generatorAccount.unconfirmedBalance + totalFee * 100L);
}
While in some places such usage may be safe, I think it's very bad practice anyway.Account generatorAccount = accounts.get(Account.getId(generatorPublicKey));
synchronized (generatorAccount) {
generatorAccount.setBalance(generatorAccount.balance + totalFee * 100L);
generatorAccount.setUnconfirmedBalance(generatorAccount.unconfirmedBalance + totalFee * 100L);
}
More to come...
The author did not even bother to run the code through static code analysis.
Also, where the hell are the JUnit tests?
How can anyone expect this kind of code to be used to handle their currency?
I do not think that this should be taken here as an opportunity to insult the developer. Please, show more humility for the work of others, even if you dont like the project.