first commit#16
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jonathanjarman
left a comment
jonathanjarman
left a comment
There was a problem hiding this comment.
In this latest build, you have an extra folder and file. This may be why the build is failing. org/example should be deleted. Then try pushing the commit to see if the build passes.
jonathanjarman
left a comment
jonathanjarman
left a comment
There was a problem hiding this comment.
This latest commit is very good. I would just try and add some comments to explain what the code is doing. I would also delete the extra Main.java file and org/example folder that are inside of this repo, as it may be why the build is not passing Travis CI. There were some extra local variables that were not being used in some of the test cases, which could be removed. Great job overall though, I noticed throughout the commit history there was steady improvement in the logic and design of the methods.
| assertThrows(IllegalArgumentException.class, () -> { | ||
| int length = manipulatedstring.count(); | ||
| assertThrows(NullPointerException.class, () -> { | ||
| int length = manipulatedstring.count(); |
There was a problem hiding this comment.
In this method int length is not needed as it won't be used. The second condition for the string is achieve from manipulatedstring.count(). you can remove "int length =" as it is not necessary.
| @@ -167,33 +169,33 @@ public void testRestoreString2() { | |||
| manipulatedstring.setString("testing"); | |||
| int[] indices = {}; | |||
| String restoredString = manipulatedstring.restoreString(indices); | |||
There was a problem hiding this comment.
You can remove the local variable String restoredString as it is not needed since the error will be thrown by the assertThrows. Picture for reference.
| for (int i = startWord; i <= endWord; i++) { | ||
| substrings[index] = words[i]; | ||
|
|
||
| for (int i = startWord - 1; i < endWord; i++) { |
There was a problem hiding this comment.
For this method rather than iterating through and copying one word at a time, you can use the arraycopy method to copy the array, which is more efficient. Picture for reference.
2 participants
inclass practise