-
Notifications
You must be signed in to change notification settings - Fork 11
Khalil coats code review #21
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
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 |
|---|---|---|
| @@ -1,32 +1,122 @@ | ||
| public class StringManipulation implements StringManipulationInterface { | ||
| //Khalil Coats | ||
| //CS 410 | ||
| //06/11/2023 | ||
| //JUnit Testing, StringManipulation implementation class | ||
|
|
||
| @Override | ||
| public class StringManipulation implements StringManipulationInterface { | ||
|
|
||
| private String objString; | ||
|
|
||
| @Override | ||
| public String getString() { | ||
| return null; | ||
| return objString; | ||
| } | ||
|
|
||
| @Override | ||
| public void setString(String string) { | ||
| objString = string; | ||
| } | ||
|
|
||
| @Override | ||
| @Override | ||
| public int count() { | ||
| return 0; | ||
|
|
||
| String[] words = this.getString().split(" +"); | ||
|
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. Good use of one-or-many whitespaces to split on, but because split() uses regex, perhaps you could go even a step farther and use "\s+" instead of " +" so that all potential characters that separate words (like newlines, whitespace, etc.) are split upon. 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. you can use \s+ instead of + so it can represent all whitespace characters |
||
| int wordCount = words.length; | ||
| return wordCount; | ||
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { | ||
| return null; | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) | ||
| { | ||
| if(n <= 0) | ||
| { | ||
| throw new IllegalArgumentException("n must be at least 1."); | ||
| } | ||
| else if(n > this.getString().length()-1) | ||
| { | ||
| throw new IndexOutOfBoundsException("index out of bounds"); | ||
|
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. The error message can be more descriptive here for if n is more than the string's length minus 1 |
||
| } | ||
| else | ||
| { | ||
| if(maintainSpacing) | ||
| { | ||
| char[] objChars = this.getString().toCharArray(); | ||
| for(int i = n; i <= this.getString().length(); i+=n) | ||
| { | ||
| objChars[i-1] = ' '; | ||
| } | ||
| this.setString(String.valueOf(objChars)); | ||
| } | ||
| else if(!maintainSpacing) | ||
|
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. Could use just a simple else here since maintainSpacing only ever has two possibilities |
||
| { | ||
| String newStr = ""; | ||
| for(int i = n-1; i < this.getString().length(); i+=(n-1)) | ||
| { | ||
| newStr = this.getString().substring(0, i) + this.getString().substring(i+1); | ||
| this.setString(newStr); | ||
| } | ||
| } | ||
| } | ||
| return this.getString(); | ||
| } | ||
|
|
||
| @Override | ||
| public String[] getSubStrings(int startWord, int endWord) { | ||
| return null; | ||
| public String[] getSubStrings(int startWord, int endWord) | ||
| { | ||
| String words[] = this.getString().split("\\W+"); | ||
| String returnList[]; | ||
| if(startWord <= 0 || endWord <= 0) | ||
| { | ||
| throw new IllegalArgumentException("Word position argument must be positive nonzero number."); | ||
| } | ||
| else if(startWord > endWord) | ||
| { | ||
| throw new IllegalArgumentException("upper bound greater than lower bound"); | ||
| } | ||
| else if(endWord > this.count()) | ||
|
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. Since you've already done the work of splitting the String into words in line 66, you could compare the length of that array instead of needing to call the count() method for error checking, but good idea to use existing methods |
||
| { | ||
| throw new IndexOutOfBoundsException("upper bound or lower is greater than word count"); | ||
|
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. In the error statement, you can add to the statement "or startWord is larger than the word count" to complete your error message. |
||
| } | ||
| else | ||
| { | ||
| returnList = new String[endWord-startWord+1]; | ||
| int loopStart = startWord; | ||
|
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. Can discard and use value of startWord in the following loop where loopStart is, since startWord is passed as an argument originally |
||
| for(int i = 0; i < endWord- startWord+1; i++) | ||
| { | ||
| returnList[i] = words[loopStart - 1 + i]; | ||
| System.out.println(returnList[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. Make sure to remove any debugging System.out.println statements that are not needed in the final code |
||
| } | ||
|
|
||
| } | ||
| return returnList; | ||
| } | ||
|
|
||
| @Override | ||
| public String restoreString(int[] indices) { | ||
| return null; | ||
| public String restoreString(int[] indices){ | ||
| String restore = ""; | ||
|
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. Really small optimization, but can throw the creation of the String to be returned after the error checking |
||
| for(int i = 0; i < indices.length; i++) | ||
| { | ||
| if(indices[i] < 0 || indices[i] >= this.getString().length()) | ||
| { | ||
| throw new IndexOutOfBoundsException("one of the indices is out of array bounds"); | ||
| } | ||
| } | ||
|
|
||
| if(indices.length != this.getString().length()) | ||
| { | ||
| throw new IllegalArgumentException("Illegal argument passed. Array argument length doesn't match string length"); | ||
| } | ||
| else | ||
| { | ||
| char[] newChars = new char[this.getString().length()]; | ||
| for(int i = 0; i <this.getString().length();i++) | ||
| { | ||
| newChars[i] = this.getString().charAt(indices[i]); | ||
| } | ||
| restore = (String.valueOf(newChars)); | ||
|
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. Using the String variable 'restore' is good for enhancing the readability of the code, but can also be left out since you could also return the 'newChars' array as a String with the same method you're using to assign it to 'restore' |
||
| } | ||
|
|
||
| return restore; | ||
| } | ||
|
|
||
|
|
||
|
|
||
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.
Good use of private access modifier to ensure encapsulation of class String member