-
Notifications
You must be signed in to change notification settings - Fork 26
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
[#25] - Added answers for conclusion #73
Closed
Closed
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,31 +281,224 @@ Here are some quick questions you can try to answer based on your execution path | |
instead? What exceptions do you think will be thrown (if any), where | ||
will the exceptions be thrown and where will they be handled? | ||
|
||
1. `redit 1 n/Alice Yu` | ||
<panel header="1. `redit 1 n/Alice Yu`" no-close> | ||
|
||
2. `edit 0 n/Alice Yu` | ||
* Exception Thrown: ParseException | ||
* Reason: Unknown command as the command word `redit` is not recognized. | ||
* Where the exception is thrown: `AddressBookParser#parseCommand()` | ||
|
||
3. `edit 1 n/Alex Yeoh` | ||
```java {.line-numbers highlight-lines="7"} | ||
public Command parseCommand(String userInput) throws ParseException { | ||
... | ||
switch (commandWord) { | ||
... | ||
default: | ||
logger.finer("This user input caused a ParseException: " + userInput); | ||
throw new ParseException(MESSAGE_UNKNOWN_COMMAND); | ||
} | ||
} | ||
``` | ||
|
||
* Where the exception is handled: `MainWindow#executeCommand()` | ||
|
||
```java {.line-numbers highlight-lines="3"} | ||
private CommandResult executeCommand(String commandText) throws CommandException, ParseException { | ||
... | ||
} catch (CommandException | ParseException e) { | ||
logger.info("An error occurred while executing command: " + commandText); | ||
resultDisplay.setFeedbackToUser(e.getMessage()); | ||
throw e; | ||
} | ||
} | ||
``` | ||
</panel> | ||
|
||
<panel header="2. `edit 0 n/Alice Yu`" no-close> | ||
|
||
* Exception Thrown: ParseException | ||
* Reason: Invalid command format as index 0 is not a non-zero unsigned integer. | ||
* Where the exception is thrown: `EditCommandParser#parse()` | ||
|
||
```java {.line-numbers highlight-lines="6"} | ||
public EditCommand parse(String args) throws ParseException { | ||
... | ||
try { | ||
index = ParserUtil.parseIndex(argMultimap.getPreamble()); | ||
} catch (ParseException pe) { | ||
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE), pe); | ||
} | ||
... | ||
} | ||
``` | ||
|
||
* Where the exception is handled: `MainWindow#executeCommand()` | ||
</panel> | ||
|
||
4. `edit 1` | ||
<panel header="3. `edit 1 n/Alex Yeoh`" no-close> | ||
|
||
5. `edit 1 n/アリス ユー` | ||
* No exception thrown. | ||
* Reason: "1" is a valid index in the person list. The command is correctly formatted and will edit the name of the person at index 1 to "Alex Yeoh". | ||
</panel> | ||
|
||
6. `edit 1 t/one t/two t/three t/one` | ||
<panel header="4. `edit 1`" no-close> | ||
|
||
* Exception Thrown: ParseException | ||
* Reason: At least one field to edit must be provided. | ||
* Where the exception is thrown: `EditCommandParser#parse()` | ||
|
||
```java {.line-numbers highlight-lines="4"} | ||
public EditCommand parse(String args) throws ParseException { | ||
... | ||
if (!editPersonDescriptor.isAnyFieldEdited()) { | ||
throw new ParseException(EditCommand.MESSAGE_NOT_EDITED); | ||
} | ||
... | ||
} | ||
``` | ||
|
||
* Where the exception is handled: `MainWindow#executeCommand()` | ||
</panel> | ||
|
||
<panel header="5. `edit 1 n/アリス ユー`" no-close> | ||
|
||
* Exception Thrown: ParseException | ||
* Reason: Names should only contain alphanumeric characters and spaces, and it should not be blank. | ||
* Where the exception is thrown: `ParserUtil#parseName()` | ||
|
||
```java {.line-numbers highlight-lines="4"} | ||
public static Name parseName(String name) throws ParseException { | ||
... | ||
if (!Name.isValidName(trimmedName)) { | ||
throw new ParseException(Name.MESSAGE_CONSTRAINTS); | ||
} | ||
... | ||
} | ||
``` | ||
|
||
* Where the exception is handled: `MainWindow#executeCommand()` | ||
</panel> | ||
|
||
<panel header="6. `edit 1 t/one t/two t/three t/one`" no-close> | ||
|
||
* No exception thrown. | ||
* Reason: The command is correctly formatted and will edit the tags of the person at index 1 to "one", "two" and "three". | ||
|
||
<panel header="Why didn’t the second tag with value “one” get added?"> | ||
|
||
Duplicate values are handled by the `ParserUtil#parseTags()` method because tags are added to a HashSet. The HashSet class inherently handles duplicates by not allowing any equal elements to be added. Therefore, any duplicate tags are not added.<br> | ||
Read more on the `add` method of the HashSet class <a href="https://docs.oracle.com/javase/8/docs/api/java/util/HashSet.html#add-E-">here</a>. | ||
</panel> | ||
|
||
</panel> | ||
|
||
<br> | ||
|
||
2. What components will you have to modify to perform the following | ||
enhancements to the application? | ||
|
||
1. Make command words case-insensitive | ||
<panel header="1. Make command words case-insensitive" no-close> | ||
|
||
2. Allow `delete` to remove more than one index at a time | ||
1. Modify `AddressBookParser#parseCommand()` to convert command words to lowercase before parsing. | ||
|
||
3. Save the address book in the CSV format instead | ||
```java {.line-numbers highlight-lines="3['.toLowerCase()']"} | ||
public Command parseCommand(String userInput) throws ParseException { | ||
... | ||
final String commandWord = matcher.group("commandWord").toLowerCase(); | ||
final String arguments = matcher.group("arguments"); | ||
} | ||
``` | ||
</panel> | ||
|
||
<panel header="2. Allow `delete` to remove more than one index at a time" no-close> | ||
|
||
4. Add a new command | ||
1. Modify `DeleteCommandParser` to parse a list of indices. | ||
2. Update `DeleteCommand` to take in a list of indices. | ||
|
||
<box type="info" seamless> | ||
|
||
Remember to update other usages of `DeleteCommand` class to handle the change in type of Index. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, really comprehensive answers here! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, done! |
||
</box> | ||
</panel> | ||
|
||
5. Add a new field to `Person` | ||
<panel header="3. Save the address book in the CSV format instead" no-close> | ||
|
||
6. Add a new entity to the address book | ||
1. Import the following classes: | ||
```java | ||
import java.io.FileWriter; | ||
import java.io.PrintWriter; | ||
``` | ||
1. Modify the `JsonAddressBookStorage#saveAddressBook()` method | ||
```java | ||
public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { | ||
requireNonNull(addressBook); | ||
requireNonNull(filePath); | ||
|
||
FileUtil.createIfMissing(filePath); | ||
|
||
try (PrintWriter out = new PrintWriter(new FileWriter(filePath.toFile()))) { | ||
out.println("Name,Phone,Email,Address,Tags"); // CSV header | ||
addressBook.getPersonList().forEach(person -> { | ||
out.println( | ||
escapeField(person.getName().toString()) + "," + | ||
escapeField(person.getPhone().toString()) + "," + | ||
escapeField(person.getEmail().toString()) + "," + | ||
escapeField(person.getAddress().toString()) + "," + | ||
escapeField(person.getTags().toString()) | ||
); | ||
}); | ||
} catch (IOException e) { | ||
logger.severe("Failed to save address book to " + filePath + ": " + e.getMessage()); | ||
throw e; | ||
} | ||
} | ||
``` | ||
1. Add a helper method to handle special characters in the fields for Person. | ||
```java | ||
private String escapeField(String field) { | ||
if (field.contains(",") || field.contains("\"")) { | ||
field = field.replace("\"", "\"\""); | ||
field = "\"" + field + "\""; | ||
} | ||
return field; | ||
} | ||
``` | ||
</panel> | ||
|
||
<panel header="4. Add a new command" no-close> | ||
|
||
1. Add a class for your new command in the [`logic.commands`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/logic/commands) package. | ||
1. Add a class for your parser to parse the new command in the [`seedu.address.logic.parser`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/logic/parser) package. | ||
1. Update `AddressBookParser` to use the new parser. | ||
|
||
<box type="info" seamless> | ||
For a more detailed explanation, refer to <a href="https://se-education.org/guides/tutorials/ab3AddRemark.html">[AB3 Tutorial: Adding a Command]</a> | ||
</box> | ||
</panel> | ||
|
||
<panel header="5. Add a new field to `Person`" no-close> | ||
|
||
1. Add a new class for the field in [`seedu.address.model.person`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/model/person). | ||
2. Update the [`Person`](https://github.com/se-edu/addressbook-level3/blob/master/src/main/java/seedu/address/model/person/Person.java) class to include the new field. | ||
3. Update the [`JsonAdaptedPerson`](https://github.com/se-edu/addressbook-level3/blob/master/src/main/java/seedu/address/storage/JsonAdaptedPerson.java) class in [`seedu.address.storage`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/storage) to include the new field. | ||
|
||
<box type="info" seamless> | ||
|
||
Remember to update other usages of `Person` class to handle the new field. | ||
</box> | ||
</panel> | ||
|
||
<panel header="6. Add a new entity to the address book" no-close> | ||
|
||
For instance, if we are adding `Event` as the new entity to the address book: | ||
1. Add a new class `Event` in `seedu.address.model.event` to represent an Event entity. | ||
1. Add a new class `AddEventCommand` in [`seedu.address.logic.commands`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/logic/commands) that is similar to AddCommand. | ||
1. Implement a `AddEventCommandParser` parser in [`seedu.address.logic.parser`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/logic/parser) to parse the relevant arguments. | ||
1. Update the [`Model`](https://github.com/se-edu/addressbook-level3/blob/master/src/main/java/seedu/address/model/Model.java) interface to add in new methods such as `addEvent` and `hasEvent`. | ||
1. Update the [`ModelManager`](https://github.com/se-edu/addressbook-level3/blob/master/src/main/java/seedu/address/model/ModelManager.java) to implement these new methods. | ||
1. Update the [`AddressBook`](https://github.com/se-edu/addressbook-level3/blob/master/src/main/java/seedu/address/model/AddressBook.java) class to create methods like `setEvent` or `getEventList` etc. | ||
1. Create a `UniqueEventList` class in `seedu.address.model.event` to handle a list of unique events. | ||
1. Implement a `JsonAdaptedEvent` in [`seedu.address.storage`](https://github.com/se-edu/addressbook-level3/tree/master/src/main/java/seedu/address/storage) to save the data in JSON format. | ||
</panel> | ||
<br> | ||
|
||
[:fas-arrow-up: **ToC**](ab3.md) | <span class="badge rounded-pill bg-primary">**++What's next?++**</span> [:fas-arrow-right: **Adding a Command**](ab3AddRemark.md) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 instead of a panel inside a panel, it would be better if the additional information was presented in an info box instead.
And the question could be made clearer: Why aren't there two tags with value "one" after this command is executed?
Lastly, would this be more accurate?
... ParserUtil#parseTags() method when tags are added to a HashSet...
What do you think?
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.
Thanks for the suggestion! Agreed that an info box would be clearer for the reader. Made the changes