-
Notifications
You must be signed in to change notification settings - Fork 6
updated the skeleton for Code Review Activity #14
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,33 +1,134 @@ | ||
|
|
||
| import java.util.Arrays; | ||
|
|
||
| public class StringManipulation implements StringManipulationInterface { | ||
|
|
||
| private String stringToManipulate; | ||
|
|
||
| @Override | ||
| public String getString() { | ||
| return null; | ||
| return stringToManipulate; | ||
| } | ||
|
|
||
| @Override | ||
| public void setString(String string) { | ||
| stringToManipulate = string; | ||
| } | ||
|
|
||
| @Override | ||
| public int count() { | ||
| return 0; | ||
|
|
||
| // if the current stringToManipulate is null, throws a NullPointerException | ||
| checkForNullString(); | ||
|
|
||
| // If the stringToManipulate is an empty string, returns 0 | ||
| if(stringToManipulate.length()==0){ | ||
| return 0; | ||
| } | ||
|
|
||
| // split the string by whitespace(s), period, comma, and other symbols | ||
| // and stores each word into a String array | ||
| String[] words = stringToManipulate.split("[\\s.(),:?!]+"); | ||
|
|
||
| // returns the length of array as the word count | ||
| return words.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. Great work on the count() method! I appreciate the clear and concise logic you've implemented. The conditional statements effectively handle scenarios where the stringToManipulate is null or empty, ensuring the method behaves correctly and avoids any potential issues. I also commend your use of regular expressions in splitting the string into words. It's a clever approach that allows for flexibility in handling different symbols and punctuation marks. Regex is very complicated, yet very useful.
Author
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. Thank you, Matthew, for the compliment. I agree regex expressions are really useful in this type of situation :) I'm glad we had a chance to learn it in the CS program. |
||
| } | ||
|
|
||
| @Override | ||
| public String removeNthCharacter(int n, boolean maintainSpacing) { | ||
| return null; | ||
| // if the current stringToManipulate is null, throws a NullPointerException | ||
| checkForNullString(); | ||
|
|
||
| // If the given n is smaller or equal to 0, throws IllegalArgumentException | ||
| if(n <= 0){ | ||
| throw new IllegalArgumentException("n must be greater than zero."); | ||
| } | ||
| // If the given n is larger than the length of the string, throws IndexOutOfBoundsException | ||
| if(n > stringToManipulate.length()){ | ||
| throw new IndexOutOfBoundsException("n must be less than or equal to the length of the string."); | ||
| } | ||
|
|
||
| StringBuilder str = new StringBuilder(); | ||
| int length = stringToManipulate.length(); | ||
|
|
||
| // Traverse the entire stringToManipulate | ||
| for(int i = 1; i <= length ;i++){ | ||
| // As long as the index of stringToManipulate is NOT divisible by n, append the | ||
| // corresponding character to the str | ||
| if(i % n != 0) { | ||
| str.append(stringToManipulate.charAt(i - 1)); | ||
| } | ||
| // if the index is divisible by n AND maintainSpacing is true, append a space | ||
| else if(maintainSpacing){ | ||
| str.append(' '); | ||
| } | ||
| } | ||
|
|
||
| return str.toString(); | ||
| } | ||
|
|
||
| @Override | ||
| public String[] getSubStrings(int startWord, int endWord) { | ||
| return null; | ||
| public String[] getSubStrings(int startWord, int endWord){ | ||
|
|
||
| // if the current stringToManipulate is null, throws a NullPointerException | ||
| checkForNullString(); | ||
|
|
||
| // If the startWord or the endWord is zero or negative, OR the endWord is smaller than the startWord, | ||
| // throws an exception | ||
| if(startWord <= 0 || endWord <= 0 || startWord > endWord) | ||
| { | ||
| throw new IllegalArgumentException("Invalid argument(s) were entered."); | ||
| } | ||
|
|
||
| // Gets the word count by using split() | ||
| String[] words = stringToManipulate.split("[\\s.,_:?!]+"); | ||
|
|
||
| // If the endWord is larger than the length of string, throws an exception | ||
| if(endWord > words.length) | ||
| { | ||
| throw new IndexOutOfBoundsException("The endWord entered is out of bounds"); | ||
| } | ||
|
|
||
| // copies the words in the specified range into a new array called substring | ||
| String[] substring = Arrays.copyOfRange(words, startWord-1, endWord); | ||
|
|
||
| return substring; | ||
| } | ||
|
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. Great job on the getSubStrings method! Appreciate how you've implemented the necessary validations and error handling. The code is clear and easy to understand, thanks to the informative exception messages you've provided. It helps users quickly identify and address any issues with their input. Additionally, your use of the split() method to extract words from the string is a smart approach. It effectively handles various whitespace and punctuation characters, ensuring accurate word extraction. Refactoring Opportunity: One small refactoring opportunity I noticed is in the if condition for validating startWord and endWord. Instead of checking for startWord <= 0 || endWord <= 0, you could simplify it by using startWord < 1 || endWord < 1. This modification aligns with the subsequent comment explaining that zero values are invalid, and it enhances readability by removing unnecessary repetition.
Author
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. Thank you for your compliment and good suggestion! I really appreciate it:) While I agree with you that (startWord < 1 || endWord < 1) looks simpler and might improve readability, I would go with my original code (startWord <= 0 || endWord <= 0) this time. The reason is that it aligns with the requirement description. The requirement description for the method uses the following expressions: Having said that, I know the requirement description would not be always clear like this and your suggestion definitely opened my eyes to other ways that could be used and possibly better in a lot of situations:) |
||
|
|
||
| @Override | ||
| public String restoreString(int[] indices) { | ||
| return null; | ||
| public String restoreString(int[] indices){ | ||
|
|
||
| // if the current stringToManipulate is null, throws a NullPointerException | ||
| checkForNullString(); | ||
|
|
||
| // If the number of indices are not the same as the length of the string, | ||
| // throws an exception | ||
| if(indices.length != stringToManipulate.length()){ | ||
| throw new IllegalArgumentException("The number of indices must match the length of the string."); | ||
| } | ||
|
|
||
| StringBuilder newString = new StringBuilder(); | ||
|
|
||
| // Traverse the indices array to the end | ||
| for(int i = 0; i < indices.length; i++){ | ||
| // if the index is negative, or larger than equal to the length of the string, | ||
| // throws an exception | ||
| if(indices[i] < 0 || indices[i]>= stringToManipulate.length()){ | ||
| throw new IndexOutOfBoundsException("Each index must be between 0 and the length of the string."); | ||
| } | ||
| // otherwise append the character at the index to the new string | ||
| newString.append(stringToManipulate.charAt(indices[i])); | ||
| } | ||
| return newString.toString(); | ||
| } | ||
|
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 a StringBuilder to construct the new string is a smart move. It helps optimize performance and memory usage, especially when dealing with multiple concatenations. Here's a small suggestion for improvement: Consider creating a separate helper method to validate the indices. By isolating the index validation logic into its own method, you can enhance code readability and maintainability. Plus, if you need to reuse this logic elsewhere in your codebase, it'll be easier to do so. Overall, your code shows a solid understanding of exception handling and effective string manipulation. You're doing an excellent job! If you have any questions or need further assistance, feel free to ask. Your contributions are highly valued, and keep up the great work!
Author
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. Thank you for your comment with insightful information! I did not know the StringBuilder helps the optimization on the performance and memory usage, I'm glad to learn that from you:) And thank you for the suggestion that I could create a separate helper method. I agree it'd be a good idea for reusability and maintainability. Based on your suggestion, I created a separate helper method to check if the string is null and if so, it throws an NullPointerException. I have the same check almost in every method so it's a good idea to separate it and reuse it. For other checks, I decided to leave the code as-is because I feel my current code is clear in which exceptions are handled when validating indices within a particular method and each method has slightly different conditions for validation so I cannot really reuse them. |
||
|
|
||
| /* Helper Functions */ | ||
|
|
||
| // Helper function to check if the string is null. If so, throw an NullPointerException | ||
| private void checkForNullString(){ | ||
| if(stringToManipulate == null) { | ||
| throw new NullPointerException("The string is null."); | ||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.