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
Add customer order schedule tests #81
Add customer order schedule tests #81
Conversation
@@ -23,11 +22,7 @@ void isSamePhone() { | |||
assertTrue(IPHONEONE.isSamePhone(IPHONEONE)); | |||
|
|||
// clone -> returns true | |||
try { | |||
assertTrue(IPHONEONE.isSamePhone((Phone) IPHONEONE.clone())); |
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.
edited method signature for clone() so there is no need to catch exception anymore, changed it cuz the exception causes further implications in other classes
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.
good choice
&& otherOrder.getPhone().equals(getPhone()) | ||
&& otherOrder.getPrice().equals(getPrice()) | ||
&& otherOrder.getStatus().equals(getStatus()) | ||
&& otherOrder.getSchedule().equals(getSchedule()) |
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.
removed schedule cuz it might still be null
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 think can still include schedule. We can use this equals more universally.
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.
Generally ok, good job!
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.
Generally well done!
&& otherOrder.getPhone().equals(getPhone()) | ||
&& otherOrder.getPrice().equals(getPrice()) | ||
&& otherOrder.getStatus().equals(getStatus()) | ||
&& otherOrder.getSchedule().equals(getSchedule()) |
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 think can still include schedule. We can use this equals more universally.
public class OrderTest { | ||
|
||
private static final String VALID_PRICE = "$1021"; | ||
private static final Status VALID_STATUS = Status.CANCELLED; |
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.
gooood
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.
Good job!
// invalid contact numbers | ||
assertFalse(ContactNumber.isValidContactNumber("")); // empty string | ||
assertFalse(ContactNumber.isValidContactNumber(" ")); // spaces only | ||
assertFalse(ContactNumber.isValidContactNumber("91")); // less than 3 numbers |
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 think should test for less than 8
No description provided.