additer command error message not specific enough

[senwei01]

As shown in the picture above, not filling in the f/feedback just results in invalid command without specifying why. I think it helps the users if the error message for this is more specific

[nus-se-bot]

Team's Response

Hi, thanks for the bug report.

reason for response.NotInScope

This appears like an error message specificity complaint at first glance, but upon reflection, I realised this is a surprisingly insidious feature request that will take a lot of effort.

Let's consider how prefixes are parsed in ArtBuddy, and I believe AB3 and basically every other project in this module.

Most parsers have code like this - snippet from AddIterationCommandParser.java

        ArgumentMultimap argMultimap =
                ArgumentTokenizer.tokenize(args, PREFIX_ITERATION_DATE,
                        PREFIX_ITERATION_DESCRIPTION, PREFIX_ITERATION_IMAGEPATH, PREFIX_ITERATION_FEEDBACK);

        if (!arePrefixesPresent(argMultimap, PREFIX_ITERATION_DATE,
                PREFIX_ITERATION_DESCRIPTION, PREFIX_ITERATION_IMAGEPATH, PREFIX_ITERATION_FEEDBACK)
                || !argMultimap.getPreamble().isEmpty()) {
            throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddIterationCommand.MESSAGE_USAGE));
        }

and this is from ParserUtil.java

    /**
     * Returns true if none of the prefixes contains empty {@code Optional} values in the given
     * {@code ArgumentMultimap}.
     */
    public static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
        return Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getValue(prefix).isPresent());
    }

What does this mean? (Rationale for this being HIGH EFFORT)

All our parser commands don't actually know what are the exact fields that are missing. It also doesn't make sense to hardcode it for one feedback field either, (we would get bug reports for being inconsistent and scuffed in our error handling) - we would need to deal with all keywords to be consistent. How would we handle having error messages for specific missing keywords?

1. Hardcode all the keywords in the parser.

   1. For every of our TWENTY commands, have a long, multi-clause if statement to check each keyword. Perhaps optimise it and make the code less ugly with a HashMap for each command.
      1. This is lots of work, unclean, and will make the code look a bit stupid.

2. Modify the implementation for ParserUtil and Prefix

   1. Another way I see this being implemented, a bit cleaner and modular so our Parser related files can handle most of the work:
   2. Possible changes:
      1. Prefixes should store the display name instead of only the prefix of what they are storing.
      2. Have a method in ParserUtil.java that is similar to the arePrefixesPresent() method, but it now looks through our modified Prefixes to tell us what is missing, for appropriate display.
   3. It is still quite a lot of work. In addition, we would need to modify all of our currently declared Prefixes to have a name to be displayed properly in the error message.

As you can see, this feature request is going to require A LOT OF EFFORT.

Rationale for low value

We do not have that many prefixes. It even fits nicely in the screenshot - it should be intuitive for the user to tell that the feedback is missing.

Due to the high effort for low value, "rectifying it is less important (based on the value/effort considerations) than the work that has been done already (because it is fine to delay lower priority work until future iterations)." and this qualifies for response.NotInScope.

We hope this explanation convinces you for our response choice. :P

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

• [ ] I disagree

Reason for disagreement: [replace this with your explanation]

---