Replace StringManipulation.java and StringManipulationTest.java with …#24
Replace StringManipulation.java and StringManipulationTest.java with …#24michael-degroot wants to merge 1 commit intomainfrom
Conversation
michael-degroot
requested review from
NadavH12,
SilverSynch and
kaimmej
and removed request for
NadavH12,
SilverSynch and
kaimmej
| @Override | ||
| public void setString(String string) { | ||
| } | ||
| public void setString(String string) { manipulatedstring = string;} |
There was a problem hiding this comment.
Ok I have noted that your data member for the string in question is "manipulatedstring" this is a good name and place to begin
| if (manipulatedstring == "") { | ||
| return wordCount; | ||
| } | ||
| String trimmedString = manipulatedstring.trim(); |
There was a problem hiding this comment.
Seems like trim is a popular tool for this method implementation, looks good to me at the moment.
| } | ||
| } | ||
| } | ||
| String finalString = String.copyValueOf(newCharArray); |
There was a problem hiding this comment.
Good organization to copy things over to one final string.
| return null; | ||
| } | ||
| public String restoreString(int[] indices){ | ||
| char[] initCharArray = manipulatedstring.toCharArray(); |
There was a problem hiding this comment.
Prominent use of toCharArray, clever tool for solving these kinds of string character counting sort of problems.
| public void testRemoveNthCharacter4() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("Hello World is such an overused phrase:)"); | ||
| assertEquals("Hello Wor d is such an overus d phrase: ", manipulatedstring.removeNthCharacter(10, true)); |
There was a problem hiding this comment.
Looks like multiples of 10, good work, good idea to test for bigger jumps.
| public void testRemoveNthCharacter6() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("Hello World"); | ||
| assertEquals(" ", manipulatedstring.removeNthCharacter(1, true)); |
There was a problem hiding this comment.
This is a smart test to make, no issues, just seeing if retains the space that we told it to keep.
| int [] array; | ||
| array=new int[]{6,7,8,9,10,5,0,1,2,3,4}; | ||
| String restoreString = manipulatedstring.restoreString(array); | ||
| assertEquals(restoreString, "World Hello"); |
There was a problem hiding this comment.
Looks like a correct rearrangement. There isn't really anything I would change about your test cases.
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("Hello, World"); | ||
| String [] sStings = manipulatedstring.getSubStrings(1, 2); | ||
| assertEquals(sStings[0], "Hello" ); |
There was a problem hiding this comment.
Good work checking the shorter strings. Making sure to test all kinds of cases is important.
karninskuel
left a comment
karninskuel
left a comment
There was a problem hiding this comment.
The overall test cases and class implementations looked very solid. There is not a lot to my eyes that I would advise changing. The class implementations seemed effective and all the test cases seemed to follow along and match the outcomes of every method. Overall, good work.
|
Karan, |
2 participants
…my versions.