-
Notifications
You must be signed in to change notification settings - Fork 36
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
[W7][T09-2] Zhang Jingchen #22
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package seedu.addressbook.commands; | ||
|
||
/** | ||
* Show last command. | ||
* Set the current command inside the input bar to last command. | ||
*/ | ||
public class LastCommand extends Command{ | ||
|
||
public static final String COMMAND_WORD = "last"; | ||
|
||
public static final String MESSAGE_USAGE = COMMAND_WORD + ":\n" | ||
+ "Last command typed: \n\t"; | ||
|
||
public String lastCommand; | ||
|
||
public LastCommand(String last) { | ||
this.lastCommand = last; | ||
} | ||
|
||
@Override | ||
public CommandResult execute() { | ||
return new CommandResult(MESSAGE_USAGE + lastCommand); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,8 @@ public class Logic { | |
|
||
private StorageFile storage; | ||
private AddressBook addressBook; | ||
private String lastCommand = ""; | ||
|
||
|
||
/** The list of person shown to the user most recently. */ | ||
private List<? extends ReadOnlyPerson> lastShownList = Collections.emptyList(); | ||
|
@@ -66,15 +68,30 @@ protected void setLastShownList(List<? extends ReadOnlyPerson> newList) { | |
|
||
/** | ||
* Parses the user command, executes it, and returns the result. | ||
* Record this user command. | ||
* @throws Exception if there was any problem during command execution. | ||
*/ | ||
public CommandResult execute(String userCommandText) throws Exception { | ||
userCommandText = checkLastCommand(userCommandText); | ||
Command command = new Parser().parseCommand(userCommandText); | ||
CommandResult result = execute(command); | ||
recordResult(result); | ||
return result; | ||
} | ||
|
||
/** | ||
* Check if the command is "last". | ||
* If yes then append the lastCommand for later execution. | ||
*/ | ||
private String checkLastCommand(String userCommandText) throws Exception { | ||
if( userCommandText.substring(0, Math.min(userCommandText.length(), 4)).equals("last")) { | ||
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. Avoid the magical string usage like "last". You have already defined it as a constant COMMAND_WORD. So please make use of it |
||
userCommandText = userCommandText.concat(" ").concat(lastCommand); | ||
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. It seems this is logically wrong. if the user type "last" and the previous command is "add something", then you concatenate both here before parsing. But the parser will not understand the concatenated string as it is not a valid command. I hope this process should be carried out once you parse the command (the application realizes it is the command LAST). |
||
} else { | ||
lastCommand = userCommandText; | ||
} | ||
return userCommandText; | ||
} | ||
|
||
/** | ||
* Executes the command, updates storage, and returns the result. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -456,6 +456,21 @@ public void execute_find_matchesIfAnyKeywordPresent() throws Exception { | |
expectedList); | ||
} | ||
|
||
/** | ||
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. Avoid unnecessary comment blocks while making a PR |
||
* The testing function for the newly implement command -LastCommand | ||
* cannot be implemented because the result of the execution depends | ||
* on the previous executed command. | ||
|
||
@Test | ||
public void execute_LastCommand() throws Exception { | ||
|
||
} | ||
|
||
* It should return the last command user typed in previously. | ||
*/ | ||
|
||
|
||
|
||
/** | ||
* A utility class to generate test data. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,12 @@ public void listCommand_parsedCorrectly() { | |
parseAndAssertCommandType(input, ListCommand.class); | ||
} | ||
|
||
@Test | ||
public void LastCommand_parsedCorrectly() { | ||
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. Try to work on adding multiple test cases in the future. e.g. user type last without any last command available |
||
final String input = "last"; | ||
parseAndAssertCommandType(input, LastCommand.class); | ||
} | ||
|
||
@Test | ||
public void exitCommand_parsedCorrectly() { | ||
final String input = "exit"; | ||
|
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.
Interesting enhancement! Good that you have updates the user guide. But when you provide examples in the user guide try to stick to valid examples, rather than using the terms 'something' to be more professional with you work and not to confuse the user who is reading the doc.