Conversation
|
Hi Tim, your code overall looks functional, concise, and is all very readable. I think most of the tests (besides the null pointer exception tests) test all the edge cases. Since the code works, I would just recommend some refactoring for cleaner code. |
| } | ||
|
|
||
| else if (endWord > words.length) { | ||
| throw new IndexOutOfBoundsException("Error: end index is larger than the string length"); |
There was a problem hiding this comment.
I think you would be able to combine this else if statement with the one from line 79. This will help make the code shorter and will combine the same type of exception.
Ex: else if (startWord > endWord || endWord > words.length) {
throw new IndexOutOfBoundsException();
}
| public String[] getSubStrings(int startWord, int endWord) { | ||
| return null; | ||
| if (string == null) { | ||
| throw new NullPointerException("Error: string is not yet initialized"); |
There was a problem hiding this comment.
Null Pointer Exception error isn't required for this method. I'm pretty sure this is actually supposed to be an Illegal Argument Exception so I would modify this as part of that exception.
| if (indices.length != string.length()) { | ||
| throw new IllegalArgumentException("Error: string length is not equal to indices length"); | ||
| } | ||
| for (int i = 0; i < indices.length - 1; i++) { |
There was a problem hiding this comment.
This outer nested for loop seems redundant. I'm not sure why you need to run the for loop for indices.length - 1. I believe you can just combine the 2 for loops into for (int i = 0; i < indices.length; i++) and then just run the exception in your if statement for if indices[i]< 0 or indices[i]>= string length
| char[] restored = new char[indices.length]; | ||
| for (int i = 0; i < indices.length; i++) { | ||
|
|
||
| restored[i] = string.charAt(indices[i]); |
There was a problem hiding this comment.
you can run this code in the original for loop at line 108 instead of writing a whole new for loop for this portion.
| fail("Not yet implemented"); | ||
| public void testGetSubStrings2() { | ||
| manipulatedstring.setString(null); | ||
| NullPointerException testException = assertThrows(NullPointerException.class, () -> { |
There was a problem hiding this comment.
I don't think you need a test for null pointer exception since this would fall under the other exceptions. It might be best to redo this test so you can test this under illegal argument exception
2 participants
No description provided.