Conversation
YukiRivera
left a comment
YukiRivera
left a comment
There was a problem hiding this comment.
Hi, Matthew! It was great I got this opportunity to see how others solved the same problems and I feel privileged to review yours. It was a relief to see my code is not too far from yours:)
I gave you alternatives in some of my comments, but they are really for sharing ideas.
One part you would probably want to actually change is testCount4( ). Please see the details in my comment for testCount4( ).
| String[] restoredStringArray = new String[indices.length]; | ||
|
|
||
| for (int i = 0; i < indices.length; i++) { | ||
| int oldIndex = indices[i]; | ||
| if (oldIndex < 0 || oldIndex >= string.length()) { | ||
| throw new IndexOutOfBoundsException(); | ||
| } | ||
| restoredStringArray[i] = String.valueOf(string.charAt(oldIndex)); | ||
| } | ||
|
|
||
| return String.join("", restoredStringArray); |
There was a problem hiding this comment.
Your restoreString() basically has the same logic as mine and there is nothing wrong with your code:)
Just to share, instead of an array, a StringBuilder could be used as an alternative, which I used in my code. The same operation can be performed with append() for the StringBuilder, then return the string representation by using toString() on the StringBuilder.
| StringBuilder stringBuilder = new StringBuilder(); | ||
| String phrase = "the quick brown fox jumps over the lazy dog"; | ||
| int occurrences = 0; | ||
|
|
||
| for (int i = 0; i < 100; i++) { | ||
| stringBuilder.append(phrase); | ||
| stringBuilder.append("\n"); | ||
| } | ||
|
|
||
| String longString = stringBuilder.toString(); | ||
| int index = 0; | ||
| while (index < longString.length()) { | ||
| index = longString.indexOf(phrase, index); | ||
| if (index == -1) { | ||
| break; | ||
| } | ||
| occurrences++; | ||
| index += phrase.length(); | ||
| } | ||
|
|
||
| assertEquals(100, occurrences); |
There was a problem hiding this comment.
testCount4( ):
I'm sure this is a working code, but I don't think you actually used count() in it...
You might want to rewrite this test case so that it tests the count() in StringManipulation class.
You could write a test case for a long string or even for an empty string.
|
Thank you so much @YukiRivera I made the changes to fix that test case and testCount4 is passing |
YukiRivera
left a comment
YukiRivera
left a comment
There was a problem hiding this comment.
Hi Matthew, I see your testCount4( ) has been updated and resolved the issue you had. I'm submitting my review with "Approve" this time:) Thank you!
2 participants
Updated