Conversation
hat-owl
left a comment
hat-owl
left a comment
There was a problem hiding this comment.
I noticed it has build errors, and have included inline comments where appropriate. Another note, I thought your implementation of count() was very good -- using split() on spaces was very clean and efficient, and better than my solution of looping a Scanner over the whole string!
I also noticed you didn't include exception checking in the test code for any of the functions that use them. You can use assertThrows for this.
Overall this is good work, but needs some more implementation of some last things to finish it.
| @@ -0,0 +1,88 @@ | |||
| public class StringManipulation implements StringManipulationInterface { | |||
| @Override | |||
There was a problem hiding this comment.
You need to declare the string member of the StringManipulation class, since it isn't inherited from the interface.
| public class StringManipulation implements StringManipulationInterface { | ||
| @Override | ||
| public String getString() { | ||
| if (this.string != null) { |
There was a problem hiding this comment.
Considering corner cases is good, but here the check for this.string is redundant and can be removed -- if this.string is null, then returning this.string will return null anyway (hence the check is unnecessary).
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { |
There was a problem hiding this comment.
The specification requires removing every n'th character (n, 2n, 3n, etc until end of string), this just removes the first one. Some ideas for this are recursive calls on removeNthCharacter with different n values, or a loop backwards, to make the multiple-of-n in case of removing characters instead of substituting them easier.
| } | ||
|
|
||
| @Test | ||
| public void testRemoveNthCharacter7() { |
There was a problem hiding this comment.
You need to test for exception handling for methods with exceptions in the specification. You can use assertThrows() for this.
2 participants
No description provided.