nus-cs2103-AY2223S2 / pe-dev-response

1 stars 0 forks source link

Phrasing for user guide delete-field could be more specific, easy to misunderstand #2966

Open nus-pe-bot opened 1 year ago

nus-pe-bot commented 1 year ago

Screenshot 2023-04-14 at 2.54.08 PM.png

Screenshot 2023-04-14 at 2.54.14 PM.png

The user guide says" the delete-field command deletes all internship entries that matches with at least one value for every field type that is specified." Again, same as the find command phrasing in the userguide, the field type that is "specified", from my understanding would be from the user input. Thus if I input delete-field n/apple t/aws, it should delete internships at apple or internships that use aws.

I put this as low severity since it is less common to delete internships using more than 1 field (probably only delete those that are rejected) than to find an internship based on more than 1 field.


[original: nus-cs2103-AY2223S2/pe-interim#3901] [original labels: severity.Low type.DocumentationBug]

seadragon2000341 commented 1 year ago

Team's Response

Hello! Thanks for writing in. I believe your main concern is regarding our implementation instead of our explanation.

In your last line, you mentioned that it is less common to delete internships using more than 1 field. For example, in delete-field n/Apple t/AWS, you were hoping that it would delete internships with company name Apple OR internships with a tag AWS. However, instead of what you expect, our command deletes internships with a company name Apple AND a tag AWS.

Clearly, there is a difference in our chosen design implementation and your proposed design implementation. Each design implementation has its own pros and cons, and we have to decide on one. This inevitably incurs a trade-off in one way or another. Your suggested implementation was actually considered by us. You can refer to the Developer Guide Page 32 for a detailed explaination.

image.png


Consider the case where you want to delete an internship with company name Apple and tag AWS. Based on your suggestion, if there is an entry with company name Apple but does not have a tag AWS, it will be unintentionally deleted through delete-field n/Apple t/AWS. Hence, this implementation does not allow you to achieve this: delete internships with company name Apple and tag AWS

Now consider the case where you want to delete an internship with company name Apple or tag AWS. With our implementation, you can do delete-field n/Apple to delete those entries with company names Apple, then do delete-field t/AWS to delete those entries with tag AWS. While this implementation brings slightly more inconvenience, it allows you to achieve both outcomes: delete internships with company name Apple and tag AWS, as well as delete internships with company name Apple or tag AWS.

Thus, our team believes in the merits of our design choice, and would not change it in the future. In the event that you are confused about the documentation and requries it to be clearer, we have actually provided many examples for the find command (in user guide pages 20 - 22) and for the delete-field command (in user guide page 24). In fact, there is a detailed explanation on page 21 - 22 (refer to Figure 5 and Table 7). The examples are meant to help you understand the command better, so if you are confused about how the command works, you can always view the examples in detail. In view of the above arguments, our team would not accept this as a bug. Thanks for your time!

Duplicate status (if any):

--