-
Notifications
You must be signed in to change notification settings - Fork 591
Fix duplicate files and wrong identation #2
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
base: main
Are you sure you want to change the base?
Conversation
- ArithmeticOperation.java - ArithmeticOperationTest.java Ref moar82#1
osahtout
left a comment
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
cristian-aldea
left a comment
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)); |
| 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.
ak-nichita
left a comment
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!