Removal of repeatable parameters as feature

The behavior of repeated parameters in AddCommand and EditCommand
is to take the last value used.
This can be confusing and hard to work with.
Therefore, instead of allowing repeatable parameters, it is better to
display them as a Command Error to the user.

The checking of duplicate parameter should occur before checking of
validity of individual fields. That is to suggest that suppose an Add or
Edit Command contains multiple duplicate fields that are all invalid,
duplicate field parse error will take precedence over other checking of
individual parameters.

docs/UserGuide.md

@@ -57,9 +57,6 @@ AddressBook Level 3 (AB3) is a **desktop app for managing contacts, optimized fo
 * Parameters can be in any order.<br>
   e.g. if the command specifies `n/NAME p/PHONE_NUMBER`, `p/PHONE_NUMBER n/NAME` is also acceptable.
 
-* If a parameter is expected only once in the command but you specified it multiple times, only the last occurrence of the parameter will be taken.<br>
-  e.g. if you specify `p/12341234 p/56785678`, only `p/56785678` will be taken.
-
 * Extraneous parameters for commands that do not take in parameters (such as `help`, `list`, `exit` and `clear`) will be ignored.<br>
   e.g. if the command specifies `help 123`, it will be interpreted as `help`.
 

src/main/java/seedu/address/commons/core/Messages.java

@@ -1,5 +1,10 @@
 package seedu.address.commons.core;
 
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import seedu.address.logic.parser.Prefix;
+
 /**
  * Container for user visible messages.
  */
@@ -9,5 +14,19 @@ public class Messages {
     public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
     public static final String MESSAGE_INVALID_PERSON_DISPLAYED_INDEX = "The person index provided is invalid";
     public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!";
+    public static final String MESSAGE_DUPLICATE_FIELDS =
+            "Multiple values specified for the following single-valued field(s): %s";
+    /**
+     * Formats the duplicate prefixes into an error message.
+     */
+    public static String getDuplicatePrefixesToMessage(Prefix... duplicatePrefixes) {
+        if (duplicatePrefixes.length == 0) {
+            return "";
+        }
+
+        Set<String> duplicateFields =
+                Stream.of(duplicatePrefixes).map(Prefix::toString).collect(Collectors.toSet());
 
+        return String.format(MESSAGE_DUPLICATE_FIELDS, String.join(" ", duplicateFields));
+    }
 }

src/main/java/seedu/address/logic/parser/AddCommandParser.java

@@ -38,6 +38,7 @@ public AddCommand parse(String args) throws ParseException {
             throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
         }
 
+        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);
         Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
         Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get());
         Email email = ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get());

src/main/java/seedu/address/logic/parser/ArgumentMultimap.java

@@ -5,6 +5,10 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Stream;
+
+import seedu.address.commons.core.Messages;
+import seedu.address.logic.parser.exceptions.ParseException;
 
 /**
  * Stores mapping of prefixes to their respective arguments.
@@ -57,4 +61,18 @@ public List<String> getAllValues(Prefix prefix) {
     public String getPreamble() {
         return getValue(new Prefix("")).orElse("");
     }
+
+    /**
+     * Throws a {@code ParseException} if any of the prefixes given in {@code prefixes} appeared more than
+     * once among the arguments.
+     */
+    public void verifyNoDuplicatePrefixesFor(Prefix... prefixes) throws ParseException {
+        Prefix[] duplicatedPrefixes = Stream.of(prefixes).distinct().filter(
+                prefix -> argMultimap.containsKey(prefix) && argMultimap.get(prefix).size() > 1)
+                .toArray(Prefix[]::new);
+
+        if (duplicatedPrefixes.length > 0) {
+            throw new ParseException(Messages.getDuplicatePrefixesToMessage(duplicatedPrefixes));
+        }
+    }
 }

src/main/java/seedu/address/logic/parser/EditCommandParser.java

@@ -42,7 +42,10 @@ public EditCommand parse(String args) throws ParseException {
             throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditCommand.MESSAGE_USAGE), pe);
         }
 
+        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);
+
         EditPersonDescriptor editPersonDescriptor = new EditPersonDescriptor();
+
         if (argMultimap.getValue(PREFIX_NAME).isPresent()) {
             editPersonDescriptor.setName(ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()));
         }

src/test/java/seedu/address/logic/parser/AddCommandParserTest.java

@@ -24,13 +24,18 @@
 import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
 import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
 import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;
 import static seedu.address.testutil.TypicalPersons.AMY;
 import static seedu.address.testutil.TypicalPersons.BOB;
 
 import org.junit.jupiter.api.Test;
 
+import seedu.address.commons.core.Messages;
 import seedu.address.logic.commands.AddCommand;
 import seedu.address.model.person.Address;
 import seedu.address.model.person.Email;
@@ -51,27 +56,76 @@ public void parse_allFieldsPresent_success() {
         assertParseSuccess(parser, PREAMBLE_WHITESPACE + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB
                 + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));
 
-        // multiple names - last name accepted
-        assertParseSuccess(parser, NAME_DESC_AMY + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB
-                + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));
 
-        // multiple phones - last phone accepted
-        assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_AMY + PHONE_DESC_BOB + EMAIL_DESC_BOB
-                + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));
+        // multiple tags - all accepted
+        Person expectedPersonMultipleTags = new PersonBuilder(BOB)
+                .withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND).build();
+        assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+                 + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, new AddCommand(expectedPersonMultipleTags));
+    }
 
-        // multiple emails - last email accepted
-        assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_AMY + EMAIL_DESC_BOB
-                + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));
+    @Test
+    public void parse_repeatedNonTagValue_failure() {
+        String validExpectedPersonString = NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB
+                + ADDRESS_DESC_BOB + TAG_DESC_FRIEND;
 
-        // multiple addresses - last address accepted
-        assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_AMY
-                + ADDRESS_DESC_BOB + TAG_DESC_FRIEND, new AddCommand(expectedPerson));
+        // multiple names
+        assertParseFailure(parser, NAME_DESC_AMY + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_NAME));
 
-        // multiple tags - all accepted
-        Person expectedPersonMultipleTags = new PersonBuilder(BOB).withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND)
-                .build();
-        assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
-                + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, new AddCommand(expectedPersonMultipleTags));
+        // multiple phones
+        assertParseFailure(parser, PHONE_DESC_AMY + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE));
+
+        // multiple emails
+        assertParseFailure(parser, EMAIL_DESC_AMY + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_EMAIL));
+
+        // multiple addresses
+        assertParseFailure(parser, ADDRESS_DESC_AMY + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_ADDRESS));
+
+        // invalid value followed by valid value
+
+        // invalid name
+        assertParseFailure(parser, INVALID_NAME_DESC + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_NAME));
+
+        // invalid email
+        assertParseFailure(parser, INVALID_EMAIL_DESC + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_EMAIL));
+
+        // invalid phone
+        assertParseFailure(parser, INVALID_PHONE_DESC + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE));
+
+        // invalid address
+        assertParseFailure(parser, INVALID_ADDRESS_DESC + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_ADDRESS));
+
+        // valid value followed by invalid value
+
+        // invalid name
+        assertParseFailure(parser, validExpectedPersonString + INVALID_NAME_DESC,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_NAME));
+
+        // invalid email
+        assertParseFailure(parser, validExpectedPersonString + INVALID_EMAIL_DESC,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_EMAIL));
+
+        // invalid phone
+        assertParseFailure(parser, validExpectedPersonString + INVALID_PHONE_DESC,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE));
+
+        // invalid address
+        assertParseFailure(parser, validExpectedPersonString + INVALID_ADDRESS_DESC,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_ADDRESS));
+
+        // multiple fields repeated
+        assertParseFailure(parser,
+                validExpectedPersonString + PHONE_DESC_AMY + EMAIL_DESC_AMY + NAME_DESC_AMY
+                        + ADDRESS_DESC_AMY + validExpectedPersonString,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_NAME, PREFIX_ADDRESS, PREFIX_EMAIL, PREFIX_PHONE));
     }
 
     @Test

src/test/java/seedu/address/logic/parser/EditCommandParserTest.java

@@ -16,14 +16,15 @@
 import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_FRIEND;
 import static seedu.address.logic.commands.CommandTestUtil.TAG_DESC_HUSBAND;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_AMY;
-import static seedu.address.logic.commands.CommandTestUtil.VALID_ADDRESS_BOB;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_AMY;
-import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_BOB;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_NAME_AMY;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_AMY;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_PHONE_BOB;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_FRIEND;
 import static seedu.address.logic.commands.CommandTestUtil.VALID_TAG_HUSBAND;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
 import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
 import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;
@@ -33,6 +34,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import seedu.address.commons.core.Messages;
 import seedu.address.commons.core.index.Index;
 import seedu.address.logic.commands.EditCommand;
 import seedu.address.logic.commands.EditCommand.EditPersonDescriptor;
@@ -90,10 +92,6 @@ public void parse_invalidValue_failure() {
         // invalid phone followed by valid email
         assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, Phone.MESSAGE_CONSTRAINTS);
 
-        // valid phone followed by invalid phone. The test case for invalid phone followed by valid phone
-        // is tested at {@code parse_invalidValueFollowedByValidValue_success()}
-        assertParseFailure(parser, "1" + PHONE_DESC_BOB + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS);
-
         // while parsing {@code PREFIX_TAG} alone will reset the tags of the {@code Person} being edited,
         // parsing it together with a valid tag results in error
         assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_DESC_HUSBAND + TAG_EMPTY, Tag.MESSAGE_CONSTRAINTS);
@@ -166,36 +164,34 @@ public void parse_oneFieldSpecified_success() {
     }
 
     @Test
-    public void parse_multipleRepeatedFields_acceptsLast() {
+    public void parse_multipleRepeatedFields_failure() {
+        // valid followed by invalid
         Index targetIndex = INDEX_FIRST_PERSON;
-        String userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY
+        String userInput = targetIndex.getOneBased() + INVALID_PHONE_DESC + PHONE_DESC_BOB;
+
+        assertParseFailure(parser, userInput,
+                        Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE));
+
+        // invalid followed by valid
+        userInput = targetIndex.getOneBased() + PHONE_DESC_BOB + INVALID_PHONE_DESC;
+
+        assertParseFailure(parser, userInput,
+                        Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE));
+
+        // mulltiple valid field repeated
+        userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY
                 + TAG_DESC_FRIEND + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY + TAG_DESC_FRIEND
                 + PHONE_DESC_BOB + ADDRESS_DESC_BOB + EMAIL_DESC_BOB + TAG_DESC_HUSBAND;
 
-        EditPersonDescriptor descriptor = new EditPersonDescriptorBuilder().withPhone(VALID_PHONE_BOB)
-                .withEmail(VALID_EMAIL_BOB).withAddress(VALID_ADDRESS_BOB).withTags(VALID_TAG_FRIEND, VALID_TAG_HUSBAND)
-                .build();
-        EditCommand expectedCommand = new EditCommand(targetIndex, descriptor);
-
-        assertParseSuccess(parser, userInput, expectedCommand);
-    }
+        assertParseFailure(parser, userInput,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS));
 
-    @Test
-    public void parse_invalidValueFollowedByValidValue_success() {
-        // no other valid values specified
-        Index targetIndex = INDEX_FIRST_PERSON;
-        String userInput = targetIndex.getOneBased() + INVALID_PHONE_DESC + PHONE_DESC_BOB;
-        EditPersonDescriptor descriptor = new EditPersonDescriptorBuilder().withPhone(VALID_PHONE_BOB).build();
-        EditCommand expectedCommand = new EditCommand(targetIndex, descriptor);
-        assertParseSuccess(parser, userInput, expectedCommand);
+        // multiple invalid values
+        userInput = targetIndex.getOneBased() + INVALID_PHONE_DESC + INVALID_ADDRESS_DESC + INVALID_EMAIL_DESC
+                + INVALID_PHONE_DESC + INVALID_ADDRESS_DESC + INVALID_EMAIL_DESC;
 
-        // other valid values specified
-        userInput = targetIndex.getOneBased() + EMAIL_DESC_BOB + INVALID_PHONE_DESC + ADDRESS_DESC_BOB
-                + PHONE_DESC_BOB;
-        descriptor = new EditPersonDescriptorBuilder().withPhone(VALID_PHONE_BOB).withEmail(VALID_EMAIL_BOB)
-                .withAddress(VALID_ADDRESS_BOB).build();
-        expectedCommand = new EditCommand(targetIndex, descriptor);
-        assertParseSuccess(parser, userInput, expectedCommand);
+        assertParseFailure(parser, userInput,
+                Messages.getDuplicatePrefixesToMessage(PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS));
     }
 
     @Test