Sort person command #116
Sort person command #116
Conversation
docs/DeveloperGuide.md
Outdated
| @@ -157,6 +157,23 @@ Classes used by multiple components are in the `seedu.addressbook.commons` packa | |||
| ## **Implementation** | |||
|
|
|||
| ### Implemented features | |||
| #### Contact features | |||
| **SortContact feature** | |||
| The `sort` command (class SortContactCommand) is a SortContact feature which allows a user to sort the current person list in alphabetical order permanently. Contacts that are being added to the list later will not be sorted and added to the end of the list. | |||
There was a problem hiding this comment.
i think this part is a bit confusing especially the command and features part?
(also what is the definition of feature vs command? because I've always interpreted feature as the whole aspect (e.g. contacts, tags, tasks...), and commands to be like what you can do with those features but maybe I'm wrong?)
also, i think the command name is normally named after the command word? so this could be kind of confusing
There was a problem hiding this comment.
hmmm if like this, then we might have to change all to command only
docs/DeveloperGuide.md
Outdated
| @@ -157,6 +157,23 @@ Classes used by multiple components are in the `seedu.addressbook.commons` packa | |||
| ## **Implementation** | |||
|
|
|||
| ### Implemented features | |||
| #### Contact features | |||
| **SortContact feature** | |||
| The `sort` command (class SortContactCommand) is a SortContact feature which allows a user to sort the current person list in alphabetical order permanently. Contacts that are being added to the list later will not be sorted and added to the end of the list. | |||
There was a problem hiding this comment.
should it be able to go back to chronological order? else, it feels like this should be our default setting since there's no other option
There was a problem hiding this comment.
No apparently I tried to think of ways to do it but we could not because of the fact that UI get the Filtered List instead of the Sorted List, unless we implement the undo feature?
| import seedu.address.model.Model; | ||
| import seedu.address.model.person.PersonNameComparator; | ||
|
|
||
| public class SortContactCommand extends Command { |
There was a problem hiding this comment.
previously raised, but maybe it should be SortCommand? or we change everything to fit e.g. AddContact...
| @@ -194,7 +194,7 @@ private CommandResult executeCommand(String commandText) throws CommandException | |||
| handleExit(); | |||
| } | |||
|
|
|||
| if (commandResult.isTagList()) { // TODO: Remove this in v1.3 | |||
There was a problem hiding this comment.
haha wait why did we need to remove this in the first place? i remember it was related to some hack thing?
There was a problem hiding this comment.
haha sorry let me rephrase, why was that comment there in the first place?
There was a problem hiding this comment.
Ohh erm I was really confused w the implementation initially, because I think this is not a good long-term solution but I guess for now, we should just proceed
Codecov Report
@@ Coverage Diff @@
## master #116 +/- ##
============================================
+ Coverage 69.96% 70.08% +0.12%
- Complexity 503 513 +10
============================================
Files 84 86 +2
Lines 1538 1558 +20
Branches 164 166 +2
============================================
+ Hits 1076 1092 +16
- Misses 427 430 +3
- Partials 35 36 +1
Continue to review full report at Codecov.
|
Fixes #114