Updated files, pull request contains my implementation and test cases#15
Updated files, pull request contains my implementation and test cases#15ElCapitanHaddock wants to merge 2 commits into
Conversation
|
Overall, you did not implement all the listed exceptions in at least two methods and padded your test counts with repeats of trivial success examples. |
| @Override | ||
| public String[] getSubStrings(int startWord, int endWord){ | ||
| String[] words = value.trim().split("\\s+"); | ||
| if (value.trim() == "" || endWord < startWord || startWord > words.length) throw new IllegalArgumentException(); |
There was a problem hiding this comment.
Exceptions should come with messages relevant to what occurred. This is apparent in multiple places.
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { |
There was a problem hiding this comment.
This method throws no exceptions at all!
| @Override | ||
| public int count() { | ||
| if (value.trim() == "") return 0; | ||
| return value.trim().split("\\s+").length; |
There was a problem hiding this comment.
The result of value.trim().split() is re-used several times in the code, but is calculated every time. It should be cached and regenerated whenever the string is changed.
| } | ||
|
|
||
| @Test | ||
| //tests swapping only one pair |
There was a problem hiding this comment.
This test is a trivial version of another test.
| int [] array; | ||
| array=new int[]{2,1,-3}; | ||
| assertThrows(ArrayIndexOutOfBoundsException.class, () -> { manipulatedstring.restoreString(array); }); | ||
| } |
There was a problem hiding this comment.
There is no test for the throwing of IllegalArgumentException when the provided array does not match the length of the string.
| manipulatedstring.setString("aaa"); | ||
| assertEquals(" ", manipulatedstring.removeNthCharacter(1, true)); | ||
| } | ||
|
|
There was a problem hiding this comment.
There are NO tests for the exceptions for this method.
|
TY for the code review, I appreciate the brutal honesty! |
2 participants
Title