Skip to content

Test pull request for peer review.#13

Open
SilverSynch wants to merge 11 commits intoSWE-CS410:mainfrom
SilverSynch:main
Open

Test pull request for peer review.#13
SilverSynch wants to merge 11 commits intoSWE-CS410:mainfrom
SilverSynch:main

Conversation

@SilverSynch
Copy link

No description provided.

@sarafarag sarafarag requested a review from NadavH12 June 14, 2023 19:00
@Override
public String removeNthCharacter(int n, boolean maintainSpacing) {
return null;
if (internalString == null) {

Choose a reason for hiding this comment

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

I didn't include a check for if my string is null so good job implementing this even though it wasn't specified! :)

else if (n < 1) {
throw new IllegalArgumentException("Removal count is less than 1.");
}

Choose a reason for hiding this comment

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

See my version for a simpler version of the code!

Choose a reason for hiding this comment

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

My (maintainSpacing) is pretty much the same as yours.

What I did for the (!maintainSpacing) case is that I just replaced the n'th char with an "empty char".
The for loop variable was still tracking the string index properly.

I'm not sure if that makes sense but let me know if you have any questions and I am happy to answer them, or feel free to look at my code.

*/
private void genWordListCache() {
// Splits the string by words.
// Matches any cluster of non-whitespace characters.

Choose a reason for hiding this comment

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

What I did here is first I trimmed leading and lagging whitespace using String.trim().
Then I also used String.split() but provided the regex " +" for 1 or more space characters.

This regex can be changed to "\s+" for 1 or more whitespace characters.

I found that by using the trim() method my issue with the regex matching an empty string was solved.

public int count() {
return 0;
if (wordListCache == null) {
throw new NullPointerException("No word list cache is available or string was not set!");

Choose a reason for hiding this comment

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

I did a similar thing here with using the String.split() method and then returning the length of that array for count.


String[] result = new String[endWord - startWord + 1];

for (int i = 0; i <= endWord - startWord; i++) {

Choose a reason for hiding this comment

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

Equivalent Code:
result = Arrays.copyOfRange(wordsListCache, startword - 1, endword);

}

// Test that restore arrays less than the length of the string throw IllegalArgumentException.
@Test

Choose a reason for hiding this comment

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

👍


// Test that restore arrays with indices less than 0 throw IndexOutOfBoundsException.
@Test
public void testRestoreString3()

Choose a reason for hiding this comment

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

👍

{
fail("Not yet implemented");

manipulatedstring.setString("That");

Choose a reason for hiding this comment

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

👍

@Test
public void testRestoreString5()
{
fail("Not yet implemented");

Choose a reason for hiding this comment

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

👍

Choose a reason for hiding this comment

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

Overall nice code. Code really shows that you have a strong grasp on the higher level features of Java. My advice to you is to try to keep things as simple as possible. Although a roundabout way might work I think that the people who developed these exercises have a simple solution in mind and it is our job to try and find that simple solution. Also be more creative with your test cases, always throw in some random whitespace into inputs because it's guaranteed to break shit. 👍 😄

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