[Emily_Su_CodeReview] Implement StringManipulation Class and Corresponding Unit Tests #26
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,122 @@ | ||
| import java.util.Arrays; | ||
|
|
||
| public class StringManipulation implements StringManipulationInterface { | ||
|
|
||
| private String[] wordArray; | ||
|
|
||
| @Override | ||
| public String getString() { | ||
| if (wordArray == null) { | ||
| throw new NullPointerException("String has not been set for current StringManipulation object (hint: use setString())."); | ||
| } | ||
| return String.join(" ", wordArray).trim(); | ||
| } | ||
|
|
||
| @Override | ||
| public void setString(String string) { | ||
| if (string.isBlank()) { | ||
| wordArray = new String[] { string }; | ||
| } | ||
| else { | ||
| wordArray = string.split(" "); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public int count() { | ||
| if (wordArray == null) { | ||
| return -1; | ||
| } | ||
| if (getString().isBlank()) { | ||
| return 0; | ||
| } | ||
| return wordArray.length; | ||
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { | ||
| String retVal = getString(); | ||
|
There was a problem hiding this comment. You have to convert to string here to do your manipulations which means code is somewhat coupled and dependent on itself. |
||
|
|
||
| if (n <= 0) { | ||
| throw new IllegalArgumentException("First argument is less than or equal to zero."); | ||
| } | ||
| if (n > retVal.length()) { | ||
| throw new IndexOutOfBoundsException("Passed " + n + " as first argument. " + n + " is greater than length of current StringManipulation object: " + retVal.length()); | ||
| } | ||
| if (n == 1) { | ||
| return maintainSpacing ? generateSpaces(retVal.length()) : ""; | ||
|
|
||
| } | ||
| if (maintainSpacing) { | ||
| for (int i = n - 1; i < retVal.length(); i += n) { | ||
| retVal = retVal.substring(0, i) + " " + retVal.substring(i + 1); | ||
|
There was a problem hiding this comment. this is clever use of .substring(x,y) |
||
| } | ||
| } | ||
| else { | ||
| for (int i = n - 1; i < retVal.length(); i += n) { | ||
| retVal = retVal.substring(0, i) + retVal.substring(i + 1); | ||
|
|
||
| i--; | ||
| } | ||
| } | ||
| return retVal; | ||
| } | ||
|
|
||
| /** | ||
| * Returns a string consisting of n spaces | ||
| * | ||
| * Example | ||
| * Method Call: generateSpaces(5); | ||
| * Return: " " | ||
| * | ||
| * @param n Determines the number of spaces to return | ||
| * @return String consisting of n spaces | ||
| */ | ||
| public String generateSpaces(int n) { | ||
| if (n < 0) { | ||
| throw new IllegalArgumentException("Cannot generate negative number of spaces."); | ||
| } | ||
| String retVal = ""; | ||
| for (int i = 0; i < n; i++) { | ||
| retVal += " "; | ||
| } | ||
| return retVal; | ||
| } | ||
|
|
||
| @Override | ||
| public String[] getSubStrings(int startWord, int endWord) { | ||
| if (wordArray == null) { | ||
| throw new NullPointerException("String has not been set for current StringManipulation object (hint: use setString())."); | ||
| } | ||
| if (startWord <= 0) { | ||
| throw new IllegalArgumentException("Index for startWord (first argument) must be greater than or equal to 1."); | ||
| } | ||
| if (endWord <= 0) { | ||
| throw new IllegalArgumentException("Index for endWord (second argument) must be greater than or equal to 1."); | ||
| } | ||
| if (startWord > endWord) { | ||
| throw new IllegalArgumentException("Index for startWord (first argument) must be less than index for endWord (second argument)."); | ||
| } | ||
| if (endWord > count()) { | ||
| throw new IndexOutOfBoundsException("Index of endWord is greater than word count for current StringManipulation object."); | ||
| } | ||
| return Arrays.copyOfRange(wordArray, startWord - 1, endWord); | ||
| } | ||
|
|
||
| @Override | ||
| public String restoreString(int[] indices) { | ||
| String currentString = getString(); | ||
|
There was a problem hiding this comment. You had to perform conversion here again to make manipulations |
||
| if (currentString.length() != indices.length) { | ||
| throw new IllegalArgumentException("Length mismatch between number of words in current StringManipulation object and length of given indices array."); | ||
| } | ||
| String[] retVal = new String[currentString.length()]; | ||
| for (int i = 0; i < indices.length; i++) { | ||
| if (indices[i] < 0) { | ||
| throw new IndexOutOfBoundsException("Invalid index: Element at index " + i + " in passed array is less than zero."); | ||
| } | ||
| if (indices[i] >= currentString.length()) { | ||
| throw new IndexOutOfBoundsException("Invalid index: Element at index " + i + " in passed array is GTE the length of the current StringManipulation object."); | ||
| } | ||
| retVal[indices[i]] = String.valueOf(currentString.charAt(i)); | ||
| } | ||
| return String.join("", retVal); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is more my opinion but after reviewing all of your code I think you might have done yourself a slight disservice by using an array of Strings as your private field. Throughout your code you are converting between the array to the String quite often and the state of your variable changes depending on the requirements of the method. Several of the methods have to call one another to achieve basic output. For example, your return removeNthCharacter method has to convert the string to an array and then perform manipulations on it. The only method which really saved you time by using the array of Strings field was the get substrings method but depending on the final implementation this time savings may or may not be worth it. Given the generic nature of the assignment and the number of methods we had to implement it might have been easier to work with a simple string and convert it to an array when necessary.