Skip to content

Add Ellis' code to skeleton#1

Open
EllisLempriere wants to merge 1 commit intomainfrom
Ellis_Lempriere_CodeReview
Open

Add Ellis' code to skeleton#1
EllisLempriere wants to merge 1 commit intomainfrom
Ellis_Lempriere_CodeReview

Conversation

@EllisLempriere
Copy link

No description provided.

@sarafarag sarafarag requested a review from WNeil June 14, 2023 18:16
@@ -1,149 +1,370 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good little organizational choice, definitely taking this in the future lol.

Comment on lines +59 to +87
String[] words = new String[(endWord - startWord) + 1];
int wordIndex = 0;

StringBuilder sb = new StringBuilder();
int wordCount = 0;
for (int i = 0; i < string.length() - 1; i++) {
// Case where first character is a word
if (i == 0 && !isSpaceChar(string.charAt(i))) {
wordCount++;
if (wordCount == startWord)
sb.append(string.charAt(i));

// Case where beginning of new word is found
} else if (isSpaceChar(string.charAt(i)) && !isSpaceChar(string.charAt(i + 1))) {
wordCount++;
i++;
if (wordCount >= startWord && wordCount <= endWord)
sb.append(string.charAt(i));

// Case where end of word is found
} else if (!isSpaceChar(string.charAt(i)) && isSpaceChar(string.charAt(i + 1))) {
if (wordCount >= startWord && wordCount <= endWord) {
sb.append(string.charAt(i));
words[wordIndex++] = sb.toString();
sb = new StringBuilder();
}

} else if (!isSpaceChar(string.charAt(i)) && wordCount >= startWord && wordCount <= endWord) {
sb.append(string.charAt(i));
Copy link

@WNeil WNeil Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringBuilder is a good choice, I however favored the split method of the string class using the regex \s+, this gives you an array of all the char groups between whitespace gaps, then you can just loop through that array and add desired words to your final words array. that solution is a bit shorter to write and simpler IMHO

Comment on lines +99 to +119
public String restoreString(int[] indices){
if (indices.length != string.length())
throw new IllegalArgumentException();

boolean[] indexUsed = new boolean[indices.length];
char[] chars = new char[indices.length];
for (int i = 0; i < indices.length; i++) {
if (indices[i] < 0 || indices[i] > string.length())
throw new IndexOutOfBoundsException();
if (indexUsed[indices[i]])
throw new IllegalArgumentException("Indices must be unique");

chars[indices[i]] = string.charAt(i);
indexUsed[indices[i]] = true;
}

StringBuilder sb = new StringBuilder();
for (char c : chars)
sb.append(c);

return sb.toString();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow wow wow, I really like the boolean array decision, very smart idea to catch something I did not even account for! I think you can construct a new String from a char array without needing to pass it through a StringBuilder, making a new String object with the char array as a param, otherwise really good job!

Comment on lines +97 to +105
// arrange
manipulatedString.setString("Here are spaces ");
int expected = 3;

// act
int actual = manipulatedString.count();

// assert
assertEquals(expected, actual);
Copy link

@WNeil WNeil Jun 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really small detail I noticed in your written tests is the declaration of a separate int over just using the number in the parameter, I assume this is an organizational choice due to the comments, weirded me out at first but I can definitely appreciate the tidiness after reading it haha

Comment on lines +17 to +30
if (string == null)
return 0;
if (string.length() == 0)
return 0;

int count = 0;
for (int i = 0; i < string.length() - 2; i++) {
if (i == 0 && !isSpaceChar(string.charAt(i)))
count++;
else if (isSpaceChar(string.charAt(i)) && !isSpaceChar(string.charAt(i + 1)))
count++;
}

return count;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty laborious solution, I like the meticulousness and I see the hussle, but like I say in a later comment you can use the split method of the string class with the regex "\s+", giving you a string array of everything split by whitespaces, and with that you could just return the length of that generated array, easy peasy.

Copy link

@WNeil WNeil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Conclusion I think the implementation and test cases made are very much satisfactory. The choice of using StringBuilder in the StringManipulation class let this code be very meticulous in its strategy and as a result will probably catch more edge case weirdness than the solutions I constructed, though perhaps at the slight cost of overcomplication in some scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants