Skip to content
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

Find command update #75

Merged

Conversation

yongmingyang
Copy link

@yongmingyang yongmingyang commented Oct 26, 2020

Context:
Q1: What is diabetes?
Tag: Chronic Diseases, Immunology
A1: Body unable to regulate blood sugar level

Q2: asdas
Tag: Nervous System, Immunology
A2: help

Q3: hehe
Tag: Immune System
A3: helpppppp

What needed fixing
find a/help
expected: return question 2 & 3
actual: returned all 3 questions

Why: Turns out, parser split a/help -> [a, help], and checked each answer to see if it contains "a" (resulted in Q1 being added).

Fixed: Parsed only the [help] into the predicates. Changes also made for find q/ and find t/.

close #76

@yongmingyang yongmingyang added the bug Something isn't working label Oct 26, 2020
@yongmingyang yongmingyang added this to the v1.3 milestone Oct 26, 2020
@yongmingyang yongmingyang added this to To review (for merging) in Team Project Oct 26, 2020
Copy link

@jonfoocy jonfoocy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one minor issue

@@ -18,7 +16,10 @@ public AnswerContainsKeywordsPredicate(List<String> keywords) {
@Override
public boolean test(QAndA qAndA) {
return keywords.stream()
.anyMatch(keyword -> StringUtil.containsWordIgnoreCase(qAndA.getAnswer().answer, keyword));
.anyMatch(keyword -> {
System.out.println("current keyword is: " + keyword);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra print statement?

Copy link

@jonfoocy jonfoocy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

Copy link

@joshruien joshruien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@joshruien joshruien merged commit 0104bb8 into AY2021S1-CS2103T-W15-1:master Oct 26, 2020
@yongmingyang yongmingyang moved this from To review (for merging) to Done in Team Project Nov 6, 2020
@yongmingyang yongmingyang self-assigned this Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

Find Command Bug
3 participants