-
Notifications
You must be signed in to change notification settings - Fork 11
Test pull request for peer review. #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b67fc46
a79fe35
f06832e
c146351
ac4fdfa
4964048
a0ffee0
b74b5cc
4ff236c
25fdea3
d2673c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,4 +25,6 @@ hs_err_pid* | |
|
|
||
| .idea | ||
| *.iml | ||
| target | ||
| target | ||
| .vscode/settings.json | ||
| lib/*.jar | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,136 @@ | ||
| public class StringManipulation implements StringManipulationInterface { | ||
| private String internalString; | ||
|
|
||
| // This implementation uses a cache which maintains a list of all words in the string. | ||
| // This cache is invalid at any time that the internal string is modified. | ||
| // Since right now we will have no subclasses, whether or not the cache is valid is easily | ||
| // maintained. | ||
| // In a more professional enviroment, maintaining cache validation would be mandatory. | ||
| private String[] wordListCache; | ||
|
|
||
| /** | ||
| * Generates or regenerated the internal cache of words in the string. | ||
| * You must call this method every time the internal string is modified! | ||
| */ | ||
| private void genWordListCache() { | ||
| // Splits the string by words. | ||
| // Matches any cluster of non-whitespace characters. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I did here is first I trimmed leading and lagging whitespace using String.trim(). This regex can be changed to "\s+" for 1 or more whitespace characters. I found that by using the trim() method my issue with the regex matching an empty string was solved. |
||
| // Note that split() splits BY what the regex identifies, and does not return the output of the regex. | ||
| // Alert! This returns the original string if there is no delimiter! | ||
| wordListCache = internalString.split("\\s"); | ||
|
|
||
| // Because split returns the original string on no delimiter, it incorrectly counts empty strings. | ||
| // Detect this by checking for a single-string array containing only a empty string. | ||
| if (wordListCache[0].matches("") && wordListCache.length == 1) { | ||
| wordListCache = new String[0]; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String getString() { | ||
| return null; | ||
| return internalString; | ||
| } | ||
|
|
||
| @Override | ||
| public void setString(String string) { | ||
| internalString = string; | ||
| genWordListCache(); | ||
| } | ||
|
|
||
| @Override | ||
| public int count() { | ||
| return 0; | ||
| if (wordListCache == null) { | ||
| throw new NullPointerException("No word list cache is available or string was not set!"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a similar thing here with using the String.split() method and then returning the length of that array for count. |
||
| } | ||
| return wordListCache.length; | ||
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { | ||
| return null; | ||
| if (internalString == null) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't include a check for if my string is null so good job implementing this even though it wasn't specified! :) |
||
| throw new NullPointerException("String was not set!"); | ||
| } | ||
| else if (n > internalString.length()) { | ||
| throw new IndexOutOfBoundsException("Removal count is greater than the length of the string (%s).".formatted(internalString.length())); | ||
| } | ||
| else if (n < 1) { | ||
| throw new IllegalArgumentException("Removal count is less than 1."); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my version for a simpler version of the code! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My (maintainSpacing) is pretty much the same as yours. What I did for the (!maintainSpacing) case is that I just replaced the n'th char with an "empty char". I'm not sure if that makes sense but let me know if you have any questions and I am happy to answer them, or feel free to look at my code. |
||
| StringBuilder builder; | ||
| // If we maintain spacing, we can copy the string to the builder and replace. | ||
| if (maintainSpacing) { | ||
| builder = new StringBuilder(internalString); | ||
| for(int i = 1; i * n - 1 <= builder.length(); i++) { | ||
| builder.setCharAt(i * n - 1, ' '); | ||
| } | ||
| } | ||
| // Otherwise, we need to copy bit by bit from the original in order to not lose our step. | ||
| // If we tried the same trick as with maintained spacing, we'd lose our place rapidly. | ||
| else { | ||
| builder = new StringBuilder(); | ||
| for (int i = 0; i <= internalString.length(); i++) { | ||
| if (i % n != 0) { | ||
| builder.append(internalString.charAt(i - 1)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return builder.toString(); | ||
| } | ||
|
|
||
| @Override | ||
| public String[] getSubStrings(int startWord, int endWord) { | ||
| return null; | ||
| public String[] getSubStrings(int startWord, int endWord){ | ||
| if (wordListCache == null) { | ||
| throw new NullPointerException("No word list cache is available or string was not set!"); | ||
| } | ||
| else if (endWord < startWord) { | ||
| throw new IllegalArgumentException("endWord cannot be less than startWord."); | ||
|
|
||
| } | ||
| else if (startWord < 1) { | ||
| throw new IllegalArgumentException("startWord cannot be less than 1."); | ||
|
|
||
| } | ||
| else if (endWord < 1) { | ||
| throw new IllegalArgumentException("endWord cannot be less than 1."); | ||
|
|
||
| } | ||
| else if (endWord < count()) { | ||
| throw new IndexOutOfBoundsException("endWord is greater than the number of words in the string (%s).".formatted(count())); | ||
| } | ||
|
|
||
| String[] result = new String[endWord - startWord + 1]; | ||
|
|
||
| for (int i = 0; i <= endWord - startWord; i++) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Equivalent Code: |
||
| result[i] = wordListCache[startWord - 1 + i]; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| @Override | ||
| public String restoreString(int[] indices) { | ||
| return null; | ||
| public String restoreString(int[] indices){ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😄 |
||
| if (internalString == null) { | ||
| throw new NullPointerException("String was not set!"); | ||
| } | ||
| else if (indices.length != internalString.length()) { | ||
| throw new IllegalArgumentException("Indices array must be exactly as long as the string (%s).".formatted(internalString.length())); | ||
| } | ||
|
|
||
| // Defer index checks until we're trying to build the string. | ||
| StringBuilder builder = new StringBuilder(); | ||
| for (int i = 0; i < indices.length; i++) { | ||
| if (indices[i] < 0) { | ||
| throw new IndexOutOfBoundsException("Index at %s is less than 0.".formatted(i)); | ||
| } | ||
| else if (indices[i] > internalString.length()){ | ||
| throw new IndexOutOfBoundsException("Index at %s is greater than the length of the string (%s).".formatted(i, internalString.length())); | ||
| } | ||
|
|
||
| builder.append(internalString.charAt(indices[i])); | ||
| } | ||
| return builder.toString(); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,19 +26,26 @@ public void testCount1() { | |
| assertEquals(4, length); | ||
| } | ||
|
|
||
| // Test that unset strings throw NullPointerException. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| @Test | ||
| public void testCount2() { | ||
| fail("Not yet implemented"); | ||
| assertThrows(NullPointerException.class, () -> {manipulatedstring.count();}); | ||
| } | ||
|
|
||
| // Test that empty strings have a word count of 0. | ||
| @Test | ||
| public void testCount3() { | ||
| fail("Not yet implemented"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| manipulatedstring.setString(""); | ||
| int length = manipulatedstring.count(); | ||
| assertEquals(0, length); | ||
| } | ||
|
|
||
| // Test that a string of 1 single word but a trailing space has a word count of 1. | ||
| @Test | ||
| public void testCount4() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("superfragilisticalidocious "); | ||
| int length = manipulatedstring.count(); | ||
| assertEquals(1, length); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Include test case for multi word strings, and a test case for strings with weird whitespace. |
||
| @Test | ||
|
|
@@ -53,29 +60,38 @@ public void testRemoveNthCharacter2() { | |
| assertEquals("I' b tt r ut s0 e 16 ts in th s tr n6 r gh ?", manipulatedstring.removeNthCharacter(3, true)); | ||
| } | ||
|
|
||
| // Test that removal counts greater than the length of the string throw IndexOutOfBoundsException. | ||
| @Test | ||
| public void testRemoveNthCharacter3() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("only"); | ||
| assertThrows(IndexOutOfBoundsException.class, () -> {manipulatedstring.removeNthCharacter(5, false);}); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| } | ||
|
|
||
| // Test that removal counts smaller than 1 throw IllegalArgumentException. | ||
| @Test | ||
| public void testRemoveNthCharacter4() { | ||
| fail("Not yet implemented"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| manipulatedstring.setString("Haha"); | ||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.removeNthCharacter(0, false);}); | ||
| } | ||
|
|
||
| // Test that removal removes all characters, including whitespace characters. | ||
| @Test | ||
| public void testRemoveNthCharacter5() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString(" e\t"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assertEquals(" e", manipulatedstring.removeNthCharacter(3, false)); | ||
| } | ||
|
|
||
| // Test that removal counts of 0 on strings with lengths of 0 still throw IllegalArgumentException. | ||
| @Test | ||
| public void testRemoveNthCharacter6() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString(""); | ||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.removeNthCharacter(0, false);}); | ||
| } | ||
|
|
||
| // Test that removal on unset strings throw NullPointerException. | ||
| @Test | ||
| public void testRemoveNthCharacter7() { | ||
| fail("Not yet implemented"); | ||
| assertThrows(NullPointerException.class, () -> {manipulatedstring.removeNthCharacter(2, false);}); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -87,25 +103,38 @@ public void testGeSubStrings1() { | |
| assertEquals(sStings[1], "string"); | ||
| } | ||
|
|
||
| // Test that endWord indices greater than the length of the string throw IndexOutOfBoundsException. | ||
| @Test | ||
| public void testGeSubStrings2() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("You"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assertThrows(IndexOutOfBoundsException.class, () -> {manipulatedstring.getSubStrings(1, 5);}); | ||
| } | ||
|
|
||
| // Test that startWord indices less than 1 throw IllegalArgumentException. | ||
| @Test | ||
| public void testGeSubStrings3() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("You"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.getSubStrings(0, 3);}); | ||
| } | ||
|
|
||
| // Test that endWord indices less than 1 throw IllegalArgumentException. | ||
| @Test | ||
| public void testGeSubStrings4() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("You"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.getSubStrings(-3, -1);}); | ||
| } | ||
|
|
||
| // Test that endWord indices less than the startWord indices throw IllegalArgumentException. | ||
| @Test | ||
| public void testGeSubStrings5() { | ||
| fail("Not yet implemented"); | ||
| manipulatedstring.setString("You"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.getSubStrings(2, 1);}); | ||
| } | ||
|
|
||
| // Test that substring grabs on unset strings throw NullPointerException. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| @Test | ||
| public void testGeSubStrings6() { | ||
| fail("Not yet implemented"); | ||
| assertThrows(NullPointerException.class, () -> {manipulatedstring.getSubStrings(1, 5);}); | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Include test cases for valid inputs with funky white-spacing. |
||
| @Test | ||
|
|
@@ -118,32 +147,39 @@ public void testRestoreString1() | |
| assertEquals(restoreString, "rat"); | ||
| } | ||
|
|
||
| // Test that restore arrays less than the length of the string throw IllegalArgumentException. | ||
| @Test | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| public void testRestoreString2() | ||
| { | ||
| fail("Not yet implemented"); | ||
|
|
||
| manipulatedstring.setString("That"); | ||
| int[] array = {1, 0, 2}; | ||
| assertThrows(IllegalArgumentException.class, () -> {manipulatedstring.restoreString(array);}); | ||
| } | ||
|
|
||
| // Test that restore arrays with indices less than 0 throw IndexOutOfBoundsException. | ||
| @Test | ||
| public void testRestoreString3() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| { | ||
| fail("Not yet implemented"); | ||
|
|
||
| manipulatedstring.setString("That"); | ||
| int[] array = {1, 0, 2, -1}; | ||
| assertThrows(IndexOutOfBoundsException.class, () -> {manipulatedstring.restoreString(array);}); | ||
| } | ||
|
|
||
| // Test that arrays with indices greater than or equal to the length of the string throw IndexOutOfBoundsException. | ||
| @Test | ||
| public void testRestoreString4() | ||
| { | ||
| fail("Not yet implemented"); | ||
|
|
||
| manipulatedstring.setString("That"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| int[] array = {1, 0, 2, 4}; | ||
| assertThrows(IndexOutOfBoundsException.class, () -> {manipulatedstring.restoreString(array);}); | ||
| } | ||
|
|
||
| // Test that restoration on unset strings throw NullPointerException. | ||
| @Test | ||
| public void testRestoreString5() | ||
| { | ||
| fail("Not yet implemented"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
|
||
| int[] array = {1, 0, 2, 3}; | ||
| assertThrows(NullPointerException.class, () -> {manipulatedstring.restoreString(array);}); | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall nice code. Code really shows that you have a strong grasp on the higher level features of Java. My advice to you is to try to keep things as simple as possible. Although a roundabout way might work I think that the people who developed these exercises have a simple solution in mind and it is our job to try and find that simple solution. Also be more creative with your test cases, always throw in some random whitespace into inputs because it's guaranteed to break shit. 👍 😄