Improved unit tests in dao module and added log4j dependency. #241
Conversation
System.out.println() statements. Added unit tests for doa module.
| final Customer customer1 = new Customer(1, "Adam", "Adamson"); | ||
| final Customer customer2 = new Customer(2, "Bob", "Bobson"); | ||
| final Customer customer3 = new Customer(3, "Carl", "Carlson"); | ||
| final List<Customer> customers = new ArrayList<Customer>(); |
There was a problem hiding this comment.
Nitpick: Verbosity can be reduced here by using <>.
|
Ping @themoffster |
|
Feel free to reject the changes, it makes no difference to me. The caveat to that is the .equals() and .hash() methods which I agree are incorrect and those changes should be rejected. Testing getters and setters is valid IMO. I see the argument of not wanting them in classes with a lot of class variables but this is not the case here. Finally, fields should be declared as final as that is best practice as you can see in reports from Sonarqube - with this project being a sample reference for other users, these best practices should be incorporated. Cheers
|
|
@themoffster We are not rejecting your changes, we want you to update them. We appreciate that you are putting effort in contributing better tests. As far as testing getter and setter goes, it comes to personal taste. All the other points made are valid IMO and we await your responses on them, so that we can improve further. Even we want to be better FYI #224 . |
Formatted code using this formatter: https://github.com/google/styleguide/blob/gh-pages/eclipse-java-google-style.xml Removed argument final declaration on interface Updated addCustomer logic for cases where the added customer already exists
|
closing this pull request to make changes based on feedback |
* add solution * Update question language * Update question language * Print "No difference" if no difference between dirs Co-authored-by: Luke LeVasseur <[email protected]>
No description provided.