Conversation
sarafarag
requested review from
pnarasimhan2021
and removed request for
sarafarag
pnarasimhan2021
left a comment
pnarasimhan2021
left a comment
There was a problem hiding this comment.
Everything looks on point, especially your test suite! Nice job covering all the test cases.
Monica20030707
left a comment
Monica20030707
left a comment
There was a problem hiding this comment.
I can see that all the test case is good, even covering the Exception cases.
However, in getString(), it would be nicer if you can make some minor changes to cover the null case with some if else statement. besides that, I think you are okay to merge.
I will give you a merge after your change.
jkwon100
left a comment
jkwon100
left a comment
There was a problem hiding this comment.
thanks for the review. I think I have made new push with the change of getString() method.
| public String[] getSubStrings(int startWord, int endWord) { | ||
| return null; | ||
| public String[] getSubStrings(int startWord, int endWord){ | ||
| if (startWord <= 0 || endWord <= 0 || startWord > endWord) throw new IllegalArgumentException("invalid input for index"); |
There was a problem hiding this comment.
I also did write the same things as you before I get a reviewer to say that it would be better for user if you can make the case startWord > endWord with a different if case so that you can throw a more specific error for that case.
|
To be honest, your algorithm is looks just the same like everybody, even me, and we, me and Priyanka, do say your test case covers everything include error cases to throw Exception, so I don't know how I can make 5 reviews for this :) |
3 participants
No description provided.