-
Notifications
You must be signed in to change notification settings - Fork 5
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
Delete command test, parser test and other descriptionbuilders #99
Delete command test, parser test and other descriptionbuilders #99
Conversation
# Conflicts: # src/main/java/seedu/address/model/travelplanner/Directory.java # src/main/java/seedu/address/model/travelplanner/Model.java # src/main/java/seedu/address/model/travelplanner/ModelManager.java # src/test/java/seedu/address/model/activity/ActivityTest.java # src/test/java/seedu/address/model/activity/UniqueActivityListTest.java # src/test/java/seedu/address/model/commons/NameContainsKeywordsPredicateTest.java
# Conflicts: # src/main/java/seedu/address/logic/wanderlustlogic/wanderlustcommands/EditCommand.java
EditActivityCommand command = (EditActivityCommand) parser.parseCommand(EditCommand.COMMAND_WORD | ||
+ " -activity " + INDEX_FIRST_TRAVELPLAN.getOneBased() + " " | ||
+ ActivityUtil.getEditActivityDescriptorDetails(descriptor)); | ||
assertEquals(new EditActivityCommand(INDEX_FIRST_TRAVELPLAN, descriptor), command); |
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 it is throwing an exception because the edit command ensures that at least one field is edited but the command and builder is building the exact same fields so there wasn't any editing done
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.
Ok resolved. Thanks!
|
||
TravelPlan tp = model.getFilteredTravelPlanList().get(targetIndex.getZeroBased()); | ||
final String[] splitName = tp.getName().name.split("\\s+"); | ||
model.updateFilteredTravelPlanList(new NameContainsKeywordsPredicate(Arrays.asList(splitName[0]))); |
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.
Is it because maybe more than 1 travel plans can have the same first name?
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
/** | ||
* Tests that a {@code TravelPlanObject}'s or {@code TravelPlan}'s {@code Name} matches any of the keywords given. | ||
*/ | ||
public class NameContainsKeywordsPredicate implements Predicate<Object> { | ||
public class NameContainsKeywordsPredicate implements Predicate<TravelPlan> { |
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.
Why this one change back to TravelPlan? It's not used in other place meh? like Activity all that
* @param travelPlanObject object to be tested. | ||
* @return true or false. | ||
*/ | ||
public boolean test(TravelPlanObject travelPlanObject) { |
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.
Oh I see what you did there. Works but I feel like it is not following some OOP shit or smth. But I think we come back to this later ba.
Failing some testcases still and not sure why.
Main problem of failure is this method:
where the updateFilteredTravelPlanList cant seem to filter out 1 object, so the assertEquals led to false.