[Emily_Su_CodeReview] Implement StringManipulation Class and Corresponding Unit Tests#26
[Emily_Su_CodeReview] Implement StringManipulation Class and Corresponding Unit Tests#26meelybug123 wants to merge 2 commits intomainfrom
Conversation
|
|
||
| public class StringManipulation implements StringManipulationInterface { | ||
|
|
||
| private String[] wordArray; |
There was a problem hiding this comment.
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.
| } | ||
| 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); |
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { | ||
| return null; | ||
| 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.
| 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()) : ""; |
| @Override | ||
| public String restoreString(int[] indices) { | ||
| return null; | ||
| String currentString = getString(); |
There was a problem hiding this comment.
You had to perform conversion here again to make manipulations
| @@ -2,148 +2,177 @@ | |||
| import org.junit.jupiter.api.BeforeEach; | |||
3 participants
Full assignment description