-
Notifications
You must be signed in to change notification settings - Fork 104
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
[W5.11][F10-2]Keith Tan #180
base: master
Are you sure you want to change the base?
Conversation
…s well as user guide
A good QoL enhancement |
Excellent function and neatly implemented! |
A function that can greatly improve users experience as it provides convenience. Good one! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to change all documentations, e.g. usage message for Find command, after modifying its functionality.
@@ -40,7 +40,7 @@ public static boolean isValidName(String test) { | |||
* Retrieves a listing of every word in the name, in order. | |||
*/ | |||
public List<String> getWordsInName() { | |||
return Arrays.asList(fullName.split("\\s+")); | |||
return Arrays.asList(fullName.toLowerCase().split("\\s+")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the situation where you need to retrieve the name in its original case, can you think of perhaps a better place to convert the cases?
|| | ||
|| 0 persons listed! | ||
|| 1 persons listed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can try to come up with more test cases, e.g. the keywords are in upper cases etc.
Made the Find function case insensitive, also updated user guide as well as test expected output