Debug time issues #242
Debug time issues #242
Conversation
| @@ -19,8 +18,7 @@ | |||
| @Override | |||
| public CommandResult execute(Model model) { | |||
| requireNonNull(model); | |||
| model.setTravelPlan(new TravelPlan(new Name("Dummy"), new WanderlustDate("0"), new WanderlustDate("0")), | |||
| new TravelPlan(null, null, null)); | |||
| model.setTravelPlanner(new TravelPlanner(null)); | |||
There was a problem hiding this comment.
Does this work? I think it cannot be null tho. "new TravelPlanner()" should work
| WanderlustDateTime activityDateTime = activityToCopy.getActivityDateTime(); | ||
|
|
||
| boolean isValidActivityDateTime = model.isValidActivityDate(activityDateTime, | ||
| travelPlanStartDate, travelPlanEndDate); |
There was a problem hiding this comment.
I think the date can be abstracted out, just pass the TravelPlan to isValidActivityDate() then in Model, you get the start and end date there.
| WanderlustDateTime activityDateTime = activityToMove.getActivityDateTime(); | ||
|
|
||
| boolean isValidActivityDateTime = model.isValidActivityDate(activityDateTime, | ||
| travelPlanStartDate, travelPlanEndDate); |
| throw new CommandException(MESSAGE_INVALID_STARTANDENDDATE); | ||
| } | ||
|
|
||
| boolean isValidStartDate = TravelPlan.isStartDateAfterToday(updatedStartDate); |
There was a problem hiding this comment.
Can this be put in isValidStartAndEndDate()? So, everything is checked in one method?
| throw new ParseException(MESSAGE_INVALID_STARTANDENDDATE); | ||
| } | ||
|
|
||
| if (!TravelPlan.isStartDateAfterToday(startDate)) { |
There was a problem hiding this comment.
Same as above. Or is it becos of the error message?
| * Checks if activity object date time is within the model's travel plan start and end date. | ||
| * @return true if activity date is within travel plan start date and end date range. | ||
| */ | ||
| boolean isValidActivityDate(WanderlustDateTime activityDateTime, WanderlustDate startDate, WanderlustDate endDate); |
There was a problem hiding this comment.
I think the JavaDoc for this can put like, see isValidActivityDate() something like that? I not sure tho.
There was a problem hiding this comment.
Think this will be considered as duplicate?
There was a problem hiding this comment.
I'm just concern about if this will be considered duplicates code. I think it's OK ba.
| return false; | ||
| } | ||
|
|
||
| if (endDate.getValue().isBefore(travelPlanStartDate.getValue()) |
There was a problem hiding this comment.
I think it's bad "SLAP" here. Perhaps can just do startDate.isBefore(WanderlustDate)
| WanderlustDate travelPlanStartDate = directory.getStartDate(); | ||
| WanderlustDate travelPlanEndDate = directory.getEndDate(); | ||
|
|
||
| if (activityDateTime.getValue().toLocalDate().isBefore(travelPlanStartDate.getValue()) |
There was a problem hiding this comment.
Same for all date checking logic in this class
| * too old (Etc, 1111-11-11) | ||
| */ | ||
| public static boolean isStartDateAfterToday(WanderlustDate startDate) { | ||
| return startDate.getValue().compareTo(LocalDate.now()) >= 0; |
There was a problem hiding this comment.
All these getValue() haha, kind of wanna abstract all of them out XD, but see you want or not. I think its ok? Like add a method in WanderlustDate compareTo() so can just call startDate.compareTo() instead of this, need getValue() everytime.
…o DebugTimeIssues
New changes:
Can only create travel plan with dates that are today or after today. (Prevent invalid/ancient dates etc. 1111-11-11) #210
End date cannot be earlier than start date for both travel plan and accommodation #214
Activity date must be in range of travelplan start date and end date #236
Accommodation start date and end date must be in range of travelplan start date and end date #236
UI show the correct number of days for both travel plan and accommodation #216
Sample data change all dates to start from 2021
Copy/Move command only allow activities to move to travelplan within date range. #236
Wishlist still accept any dates from activity, since it doesnt care about dates till you add it in a travelplan