Conversation
ACBxBC
left a comment
ACBxBC
left a comment
There was a problem hiding this comment.
Looks good to me.
Learned a lot from the test case you made and what I may need to update with related test cases in my implementation.
| @@ -1,33 +1,154 @@ | |||
| public class StringManipulation implements StringManipulationInterface { | |||
| import java.util.ArrayList; | |||
There was a problem hiding this comment.
Very weird that I encountered this too but only after the build was semi-successful.
I did not get the same error until I could actually build and have found that some of my test cases failed as well.
Good catch.
There was a problem hiding this comment.
Alvin, so I seemed to have misunderstood my own solution to the Travis build fail issue. The fundamental problem was my mishandling of the commit & add commands while commenting in/out the import statements. So, if I'm not mistaken again, StringManipulation() does in fact need the import statement for ArrayList and StringManipulationTest() should only have the original import statements from the skeleton, no others.
After making sure this was the case, my build passed without issues (and without my panic), thank goodness.
In your case, I agree that your build had failed because, like you said, some of your test cases did not pass, and not because of import statement issues like me.
| }); | ||
| } | ||
|
|
||
| // This will test if the method will throw an IndexOutOfBoundsException(). |
There was a problem hiding this comment.
I believe this and the other previous test case is what I accidentally cut off in both of my submissions.
Good to test the out of bounds exception on array.
There was a problem hiding this comment.
Alvin, interesting that your test cases somehow got cut off, hopefully you get those back in!
I'm curious, I know that the assignment calls for a test on for IllegalArgumentException() and IndexOutOfBoundsException(), do you think there are other possible exceptions that this method could throw? ArrayIndexOutOfBoundsException(), perhaps?
| assertEquals(restoreString, "rat"); | ||
| } | ||
|
|
||
| // This will test the method if it will swap the two words in the string. |
There was a problem hiding this comment.
Does this array index work for both whitespace character since each len() index is considered a string character?
There was a problem hiding this comment.
Alvin, thanks for your comment! Yes, if I understand your comment correctly, this array will work for whitespace characters since whitespaces are considered strings, or substrings at the specific index.
Based on your comment, I've refactored my code to add an additional test that will explicitly test the whitespace characters. Please see my next push update for the additional test that specifically addresses this comment.
| public void testRemoveNthCharacter5() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("0 3 6 9"); | ||
| assertEquals("0 ", manipulatedstring.removeNthCharacter(3, false)); |
There was a problem hiding this comment.
Good test case.
Would this work if the #n space start is changed?
I.e. 5n or 10n and so on, but may require a nested test for out of bounds I guess. Hope that somewhat makes sense.
2 participants
pull request