-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix duplicate files and wrong identation #2
base: main
Are you sure you want to change the base?
Conversation
- ArithmeticOperation.java - ArithmeticOperationTest.java Ref moar82#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Same for the one below
Integer actual = operations.addOrSub(2, 6); | |
Integer expected = 8; | |
assertEquals(expected, actual); | |
assertEquals(8, operations.addOrSub(2, 6)); |
@@ -2,8 +2,7 @@ | |||
|
|||
public class ArithmeticOperation { | |||
|
|||
public Integer addOrSub(Integer a, Integer b) | |||
{ | |||
public Integer addOrSub(Integer a, Integer b) { | |||
if (a > b) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah my bad you're right, nevermind my comment then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Unless you are a 10x developer, those changes would affect code readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
This PR closes #1.
Hope this helps with the code quality!