Skip to content
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

[W6.2f][F09-3]Sarah Taaher Bonna #129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/seedu/addressbook/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import seedu.addressbook.data.tag.Tag;

import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
Expand Down Expand Up @@ -60,7 +61,8 @@ public ReadOnlyPerson getPerson() {
public CommandResult execute() {
try {
addressBook.addPerson(toAdd);
return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd));
List<ReadOnlyPerson> allPersons = addressBook.getAllPersons().immutableListView();
return new CommandResult(String.format(MESSAGE_SUCCESS, toAdd), allPersons);
} catch (UniquePersonList.DuplicatePersonException dpe) {
return new CommandResult(MESSAGE_DUPLICATE_PERSON);
}
Expand Down
13 changes: 10 additions & 3 deletions src/seedu/addressbook/commands/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ public static String getMessageForPersonListShownSummary(List<? extends ReadOnly
/**
* Executes the command and returns the result.
*/
public CommandResult execute(){
throw new UnsupportedOperationException("This method should be implement in child classes");
}
// public CommandResult execute(){
// throw new UnsupportedOperationException("This method should be implement in child classes");
// }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't leave commented out code. Just go ahead and delete removed code, as we can recover them later


//Note: it is better to make the execute() method abstract, by replacing the above method with the line below:
//public abstract CommandResult execute();

public abstract CommandResult execute();

/**
* Supplies the data the command will operate on.
*/
Expand All @@ -70,4 +72,9 @@ public int getTargetIndex() {
public void setTargetIndex(int targetIndex) {
this.targetIndex = targetIndex;
}

public boolean isMutating(String userCommandText) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should have a header comment. If not, users of the method as well as those who overrides method will not know how exactly the method behaves. Also, as pointed out in the other PR, you should make use of polymorphism and implement the different behavior in individual sub classes

if(userCommandText == "add" || userCommandText == "delete") return true;
else return false;
}
}
5 changes: 4 additions & 1 deletion src/seedu/addressbook/commands/DeleteCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList.PersonNotFoundException;

import java.util.List;


/**
* Deletes a person identified using it's last displayed index from the address book.
Expand All @@ -30,7 +32,8 @@ public CommandResult execute() {
try {
final ReadOnlyPerson target = getTargetPerson();
addressBook.removePerson(target);
return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, target));
List<ReadOnlyPerson> allPersons = addressBook.getAllPersons().immutableListView();
return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, target), allPersons);

} catch (IndexOutOfBoundsException ie) {
return new CommandResult(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
Expand Down
6 changes: 3 additions & 3 deletions src/seedu/addressbook/ui/DarkTheme.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
-fx-font-size: 12pt;
-fx-font-family: "Consolas";
-fx-font-weight: bold;
-fx-text-fill: yellow;
-fx-text-fill: orange;
-fx-control-inner-background: derive(#1d1d1d,20%);
}

.text-area {
-fx-background-color: black;
-fx-control-inner-background: black;
-fx-background-color: pink;
-fx-control-inner-background: lightblue;
-fx-font-family: "Segoe UI Semibold";
-fx-font-size: 10pt;
-fx-padding: 5 5 5 5;
Expand Down
1 change: 1 addition & 0 deletions src/seedu/addressbook/ui/MainWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public void clearOutputConsole(){
/** Displays the result of a command execution to the user. */
public void displayResult(CommandResult result) {
clearOutputConsole();
outputConsole.setText("List of Contact(s) found: \n");
final Optional<List<? extends ReadOnlyPerson>> resultPersons = result.getRelevantPersons();
if(resultPersons.isPresent()) {
display(resultPersons.get());
Expand Down
2 changes: 1 addition & 1 deletion test/java/seedu/addressbook/logic/LogicTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void execute_add_successful() throws Exception {
assertCommandBehavior(helper.generateAddCommand(toBeAdded),
String.format(AddCommand.MESSAGE_SUCCESS, toBeAdded),
expectedAB,
false,
true,
Collections.emptyList());

}
Expand Down