Skip to content

Commit

Permalink
Merge pull request #341 from nikele2001/Branch-Update-DG
Browse files Browse the repository at this point in the history
Update DG
  • Loading branch information
nikele2001 authored Nov 14, 2023
2 parents 49587de + fca1903 commit fb1881c
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 36 deletions.
52 changes: 31 additions & 21 deletions docs/DeveloperGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ The `UI` component uses the JavaFx UI framework. The layout of these UI parts ar
that are in the `src/main/resources/view` folder. For example, the layout of the
[`MainWindow`](https://github.com/AY2324S1-CS2103T-F12-1/tp/blob/master/src/main/java/seedu/address/ui/MainWindow.java)
is specified in
[`MainWindow.fxml`](https://github.com/AY2324S1-CS2103T-F12-1/tp/blob/master/src/main/resources/view/MainWindow.fxml)
[`MainWindow.fxml`](https://github.com/AY2324S1-CS2103T-F12-1/tp/blob/master/src/main/resources/view/MainWindow.fxml).

The `UI` component,

Expand Down Expand Up @@ -161,7 +161,7 @@ chronological order.
* `observableAppointments` and `sortedAppointments` depend on `filteredPersons`. Hence, appointments listed are for
`Person` objects in `filteredPersons`.
* does not depend on any of the other three components (as the `Model` represents data entities of the domain, they
should make sense on their own without depending on other components)
should make sense on their own without depending on other components).

### Storage component

Expand Down Expand Up @@ -256,22 +256,22 @@ Alternative 2: Create a hashset of Appointments for each Person.

**Aspect: How to implement override prompt**

Alternative 1(current solution): Create a separate constructor in CommandResult to handle overriding
Alternative 1(current solution): Create a separate constructor in CommandResult to handle overriding.
- Pros:
* Quick solution to the problem
* This "freezes" functionality of the program to force user to acknowledge or cancel the execution of the command
* Easy to implement.
* This "freezes" functionality of the program to force user to acknowledge or cancel the execution of the command.
- Cons:
* This creates multiple different constructors within the CommandResult
* This creates multiple different constructors within the CommandResult.

Alternative 2: Abstract CommandResult to get a successfulExecutionResult and a PausedExecutionResult
Alternative 2: Abstract CommandResult to get a successfulExecutionResult and a PausedExecutionResult.
- Pros:
* Improves code readability and reduces coupling in code
* Improves code readability and reduces coupling in code.
- Cons:
* Time-consuming to refactor code
* Improper implementation could result in breaking of coding principles
* Improper implementation could result in breaking of coding principles.
- Note:
* With additional time, alternative 2 can be implemented by refactoring the code to create multiple subclasses. Be wary of the
liskov substitution principle when doing so. The earlier alternative 2 is implemented, the better to reduce amount of code
Liskov Substitution Principle (LSP) when doing so. The earlier alternative 2 is implemented, the better to reduce amount of code
that needs to be refactored.
* The above implementation can be done in conjunction with the clear command prompt to reduce code coupling.

Expand All @@ -288,7 +288,7 @@ The following sequence diagram illustrates how the complete operation is execute

<img src="images/CompleteSequenceDiagram.png" width="800"/>

<div markdown="span" class="alert alert-primary">:information_source: The lifeline of the diagram should end at the destroyer mark (X) but reaches end of diagram due to limitation of plantUML
<div markdown="span" class="alert alert-primary">:information_source: The lifeline of the diagram should end at the destroyer mark (X) but reaches end of diagram due to limitation of plantUML.
</div>

The following activity diagram illustrates how the complete operation is executed.
Expand Down Expand Up @@ -375,7 +375,7 @@ The enhanced find mechanism is facilitated by the `CombinedPredicate` and utilis

#### Implementation Overview

Here's a sequence diagram that demonstrates how `FindCommand` works.
Here's a sequence diagram that demonstrates how `FindCommand` works:

![FindCommandSequenceDiagram](images/FindCommandSequenceDiagram.png)

Expand Down Expand Up @@ -433,7 +433,7 @@ time. This feature is facilitated through the `SortCommand` class.

#### Implementation Overview

The following diagram summarises what happens when the user executes a sort command
The following diagram summarises what happens when the user executes a sort command:

<img src="images/SortClassActivityDiagram.png" width="700"/>

Expand Down Expand Up @@ -465,8 +465,8 @@ A `CommandResult` class is created and returned.
**Alternative 1(current choice):** User can sort by name and appointment at any time. As such, calling find on the sorted list will result
in the ordering of find to also be sorted.
- Pros: Improved usability of maintaining order of list throughout without the list having to be reordered after
each command
- Cons: Limited sorting options as of now
each command.
- Cons: Limited sorting options as of now.


### Appointment List Feature
Expand All @@ -493,11 +493,21 @@ The following sequence diagram shows how the appointment list is updated. The `s
- Cons: Errors with respect to `addressBook` will affect the appointment list rendered.

* **Alternative 2:** Implement it within `addressBook`
- Pros: `persons` and `appointmentList` are handled separately within `addressBook` and hence the appointment
list is not dependent on `persons` in `addressBook`
- Pros: `persons` and `appointmentList` are handled separately within. `addressBook` and hence the appointment
list is not dependent on `persons` in `addressBook`.
- Cons: `filteredPersons` and `sortedAppointments` might not correspond since `sortedAppointments` is no longer
dependent on `filteredPersons`.

**Aspect: How to update appointment list**
* **Alternative 1 (current choice):**
Empty `appointmentList` and populate the list when there is a command that can potentially update it.
- Pros: Easy to implement.
- Cons: Time complexity might not be optimal if the application is dealing with large amounts of data.
* **Alternative 2:**
Based on specific index that user inputs for commands, update `appointmentList` accordingly.
- Pros: Time complexity will be optimal and will not deterioriate with increasing amounts of data.
- Cons: More difficult to test.

--------------------------------------------------------------------------------------------------------------------

## **Documentation, logging, testing, configuration, dev-ops**
Expand All @@ -515,11 +525,11 @@ The following sequence diagram shows how the appointment list is updated. The `s
### Product scope

**Target user profile**: Financial Advisors
* has a need to manage a significant number of client contacts
* has a need to manage a significant number of client contacts.
* prefer desktop apps over other types
* can type fast
* prefers typing to mouse interactions
* is reasonably comfortable using CLI apps
* can type fast.
* prefers typing to mouse interactions.
* is reasonably comfortable using CLI apps.

**Value proposition**:
This tool functions as a digital address book suited to the needs of financial advisors.
Expand Down
4 changes: 2 additions & 2 deletions docs/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,12 @@ matching the one input by user, **and is regardless of the currently displayed c
- After performing the complete, the contact list displayed will be reset to display all contacts in the contact book.

<div markdown="span" class="alert alert-primary">:information_source:
Note that an appointment's date is considered to be a match with user's input `APPOINTMENT_DATE` if the **year, month and day are the same**. Time of the appointment does not matter in this command.
**Note that** an appointment's date is considered to be a match with user's input `APPOINTMENT_DATE` if the **year, month and day are the same**. Time of the appointment does not matter in this command.
</div>

Acceptable Values: Refer to [Argument Summary](#argument-summary).

<div markdown="span" class="alert alert-primary">:information_source:
<div markdown="span" class="alert alert-primary">:bulb: **Tip**
To save time, you may find it useful to `complete` by `APPOINTMENT_DATE` once at the end of the day. This will clear all your completed appointments for the entire day.
</div>

Expand Down
4 changes: 3 additions & 1 deletion docs/diagrams/AppointmentListSequenceDiagram.puml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ deactivate observableAppointments
loop through filteredPersons
ModelManager -> ModelManager: addToAppointmentListIfPresent(person)
activate ModelManager

ModelManager -> observableAppointments: add(appointment)
activate observableAppointments
deactivate observableAppointments
deactivate ModelManager
end loop

Expand Down
Binary file modified docs/images/AppointmentListSequenceDiagram.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 4 additions & 4 deletions docs/team/jylow.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ Given below are my contributions to the project.
* Justification: Most people only arrange one appointment at a time. So the design prevents multiple appointments and also serves as a reminder of a previously set appointment.
* Highlights: This feature causes the logic flow of the method to change if there is currently an appointment and results in breaking the execution into 2, getting a response from the user before deciding whether to continue the execution of the program.
* Credits: The feature was implemented by referencing the help function from AB3. However, the main logic of the function was done by myself.

* **Code Contributed**: [RepoSense](https://nus-cs2103-ay2324s1.github.io/tp-dashboard/?search=jylow&breakdown=true)

* **Enhancements Implemented**:
* **Enhancements Implemented**:
* Fixed bugs identified during manual testing.

* **Contributions to the UG**:
* Added documentation for the feature `sort`. [\#81](https://github.com/AY2324S1-CS2103T-F12-1/tp/pull/81)
* Maintained known issues to be solved.
Expand All @@ -44,5 +44,5 @@ Given below are my contributions to the project.
* Contributed to 1 forum discussion. (examples: [1](https://github.com/nus-cs2103-AY2324S1/forum/issues/172#issuecomment-1730790631))
* Reported bugs and suggestions for other teams in the class during PED.

* **Contributions to team-based tasks**:
* **Contributions to team-based tasks**:
* Released v1.3 and v1.4 of the application.
32 changes: 24 additions & 8 deletions src/main/java/seedu/address/logic/parser/GatherCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,11 @@ public GatherCommand parse(String args) throws ParseException {
}

if (argMultimap.getValue(PREFIX_FINANCIAL_PLAN).isPresent()) {
String financialPlanArgs = removePrefix(trimmedArgs, PREFIX_FINANCIAL_PLAN);
validateFinancialPlan(financialPlanArgs);
assert isValidFinancialPlanName(financialPlanArgs) : "Prompt has to meets valid FP requirements";
return new GatherCommand(new GatherEmailByFinancialPlan(financialPlanArgs));
return createGatherByFinancialPlan(trimmedArgs);
}

if (argMultimap.getValue(PREFIX_TAG).isPresent()) {
String tagArgs = removePrefix(trimmedArgs, PREFIX_TAG);
validateTag(tagArgs);
assert isValidTagName(tagArgs) : "Prompt has to meets valid Tag requirements";
return new GatherCommand(new GatherEmailByTag(tagArgs));
return createGatherByTag(trimmedArgs);
}

throw new ParseException(
Expand All @@ -59,4 +53,26 @@ public String removePrefix(String currArg, Prefix prefix) {
String trimmedArg = noPrefix.trim();
return trimmedArg;
}

/**
* Creates a GatherCommand object for Financial Plan from {@code argString}
*/
private GatherCommand createGatherByFinancialPlan(String argString) throws ParseException {
String financialPlanArg = removePrefix(argString, PREFIX_FINANCIAL_PLAN);
validateFinancialPlan(financialPlanArg);
assert isValidFinancialPlanName(financialPlanArg) : "Prompt has to meets valid FP requirements";
GatherEmailByFinancialPlan prompt = new GatherEmailByFinancialPlan(financialPlanArg);
return new GatherCommand(prompt);
}

/**
* Creates a GatherCommand object for Tag from {@code argString}
*/
private GatherCommand createGatherByTag(String argString) throws ParseException {
String tagArg = removePrefix(argString, PREFIX_TAG);
validateTag(tagArg);
assert isValidTagName(tagArg) : "Prompt has to meets valid Tag requirements";
GatherEmailByTag prompt = new GatherEmailByTag(tagArg);
return new GatherCommand(prompt);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,6 @@ public void equals() {
assertTrue(fp1.equals(fp1));
assertTrue(fp1.equals(fp2));
assertFalse(fp1.equals(fp3));
assertFalse(fp2.equals(null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -206,4 +206,10 @@ public void asUnmodifiableObservableList_modifyList_throwsUnsupportedOperationEx
public void toStringMethod() {
assertEquals(uniquePersonList.asUnmodifiableObservableList().toString(), uniquePersonList.toString());
}

@Test
public void equals() {
assertEquals(uniquePersonList, uniquePersonList);
assertFalse(uniquePersonList.equals(null));
}
}

0 comments on commit fb1881c

Please sign in to comment.