Conversation
- ArithmeticOperation.java - ArithmeticOperationTest.java Ref moar82#1
cristian-aldea
left a comment
There was a problem hiding this comment.
Looks great! Thank you for working on this. However, I had some more suggestions if its not too much to ask
| Integer actual = operations.addOrSub(2, 6); | ||
| Integer expected = 8; | ||
| assertEquals(expected, actual); |
There was a problem hiding this comment.
💡 Same for the one below
| Integer actual = operations.addOrSub(2, 6); | |
| Integer expected = 8; | |
| assertEquals(expected, actual); | |
| assertEquals(8, operations.addOrSub(2, 6)); |
| public Integer addOrSub(Integer a, Integer b) | ||
| { | ||
| public Integer addOrSub(Integer a, Integer b) { | ||
| if (a > b) { |
There was a problem hiding this comment.
Might as well shorten this code in the interest of quality :^)
Instead of
if (a > b) {
return a - b;
} else {
return a + b;
}
Do
return a > b ? a - b : a + b;
There was a problem hiding this comment.
Might as well shorten this code in the interest of quality :^)
Instead of
if (a > b) { return a - b; } else { return a + b; }Do
return a > b ? a - b : a + b;
Not agree with this. The use of the ternary operation affects code readability is not a recommended practice.
There was a problem hiding this comment.
Ah my bad you're right, nevermind my comment then
There was a problem hiding this comment.
I agree. Unless you are a 10x developer, those changes would affect code readability.
This PR closes #1.
Hope this helps with the code quality!