NicholasTanYY / pe

0 stars 0 forks source link

No clear purpose of having the command addtwdc #6

Open NicholasTanYY opened 4 months ago

NicholasTanYY commented 4 months ago

Since the addtask command's objective is already to add a new task into the current user's list, it might inconvenience the user to use another command such as addtwdc to add tasks. Addtask should already check if the task is a duplicate, rendering the other command addtwdc to be not as effective.

nus-pe-bot commented 4 months ago

Team's Response

No details provided by team.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

addtask recognises duplicate when task does not exist

image.png

image.png

I am unable to add a task that clashes with any existing task. This may pose a problem, for example if I have an event from 11am to 3pm and I may need to run a quick errand from 11.30am to 12pm. I will not be able to add that into my timetable to track it.


[original: nus-cs2113-AY2324S2/pe-interim#2560] [original labels: type.FeatureFlaw severity.High]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

This is what we purposely designed such that no 2 event happens concurrently.

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: My issue: TLDR~ The existence of the addtwdc command is questionable (or pointless). Duplicate handling should be done in the other add methods instead.

Their issue: TLDR~ The user cannot have multiple events at the same time.


## :question: Issue response Team chose [`response.Rejected`] - [x] I disagree **Reason for disagreement:** This is not a duplicate issue to the one that was flagged.
## :question: Issue severity Team chose [`severity.High`] Originally [`severity.Low`] - [x] I disagree **Reason for disagreement:** The severity should be Medium (changed from Low initially). All commands to add tasks (including the most basic addtask command) should already have duplicate exception handling. The following code from their Github page is the main logic behind the addtwdc command (I cannot manually test their addtwdc command to determine its behaviour as their addtwdc command failed to run correctly during the PE): ``` public boolean addUserTaskWithDuplicationCheck(String dayOfWeek, Task task) { String capitalizedDay = dayOfWeek.substring(0, 1).toUpperCase() + dayOfWeek.substring(1); ArrayList tasksOfDay = weeklyTasks.get(capitalizedDay); for (Task existingTask : tasksOfDay) { if (existingTask.getDescription().equalsIgnoreCase(task.getDescription()) && existingTask.getStartTime().equals(task.getStartTime()) && existingTask.getEndTime().equals(task.getEndTime())) { return false; } } tasksOfDay.add(task); return true; } ``` From the code, we can see that the method identifies the new task as a duplicate when the new task has the same description , the same start time, and the same end time as an existing task, which means that this command is checking if the user is trying to add the exact same task at the exact same duration in the day. It then does not make sense that the other add commands allow these duplicates. This is a flaw in the feature of the application. Code from their repo can be found here https://github.com/AY2324S2-CS2113-T13-2/tp/blob/master/src/main/java/seedu/duke/Timetable.java#L51