Event sourcing #610
Event sourcing #610
Conversation
|
@radresian can you resolve the conflict please and I will review. |
|
@iluwatar conflict is resolved, also I added some comments on App class, added a Unit Test and replaced System.otuts with log4j logging. |
|
|
||
| /** | ||
| * Created by Serdar Hamzaogullari on 06.08.2017. | ||
| */ |
There was a problem hiding this comment.
Can you add proper descriptions for the classes? What are the classes responsibilities. This occurs elsewhere too.
| */ | ||
| public class AccountService { | ||
|
|
||
| private EventProcessor eventProcessor; |
There was a problem hiding this comment.
Fields set only on construction should be final. Check elsewhere too.
| public void depositMoney(int accountNo, BigDecimal money) { | ||
| MoneyDepositEvent moneyDepositEvent = new MoneyDepositEvent( | ||
| SequenceIdGenerator.nextSequenceId(), new Date().getTime(), accountNo, money); | ||
| eventProcessor.process(moneyDepositEvent); |
There was a problem hiding this comment.
Seems unnecessary to create the temp variable. Could be written on one line. Check elsewhere for this too.
| return sequenceId; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
This class is not meant to be instantiated, right? It should have private constructor.
| @@ -0,0 +1,115 @@ | |||
| /** | |||
There was a problem hiding this comment.
The path of the test file is funny. It's usually src/test/java, right? At the moment mvn clean install doesn't run the test at all.
|
@radresian review comments given. Please let me know once you've implemented the changes. |
…ample - final fields are marked as final - removed unnecessary temp variables - added private constructor for not instantiated static class AccountAggregate - path of te test file is corrected
|
@iluwatar your change requests are implemented. I removed all optional classes and interfaces to simplify my example. |
|
Looks perfect @radresian Thank for for the new pattern 👍 |
2 participants
No description provided.