Skip to content

Update skeleton#3

Open
carector wants to merge 2 commits into
mainfrom
Caleb_Rector_CodeReview
Open

Update skeleton#3
carector wants to merge 2 commits into
mainfrom
Caleb_Rector_CodeReview

Conversation

@carector

Copy link
Copy Markdown

Update implementation for StringManipulation and StringManipulationTest

@sarafarag
sarafarag requested a review from bpsix June 15, 2023 18:03
@bpsix

bpsix commented Jun 15, 2023

Copy link
Copy Markdown

Overall looks good. You use the split() method throughout, checking for newlines and space characters. I also did this, but apparently \s covers tabs, newlines, carriage returns and a few more characters. So, like my code, the regex can be replaced with just "\s".

In your restoreString() method, I like how you used 1 loop to simultaneously check for invalid indices and, if they are valid, just update the to-be returned string. I opted for two loops, checking for valid indices first, but your solution is more efficient.

One thing that may help in the future is adding more comments so your thought process can be followed with ease.

if(s.length() == 0)
return 0;

String[] arr = s.split("[\\s\\n]");

@bpsix bpsix Jun 15, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can be simplified as: String[] arr = s.split("\\s+")

There should be a '+' in case there are multiple spaces in between words

int length = manipulatedstring.count();
assertEquals(4, length);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have you considered adding a test case that checks the null string case?

}

// Test whether exceptions are thrown when startWord > endWord or when one of the variables is < 0
@Test

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In Java 8+, you can use Lamba expressions with JUnit to handle exceptions. Here's a resource that shows how this is done: https://blog.codeleak.pl/2014/07/junit-testing-exception-with-java-8-and-lambda-expressions.html

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks! I've been meaning to work more with lambda expressions. I'll be sure to check out your implementation as well.


// Tests whether an exception is thrown when endWord is out of bounds
@Test
public void testGeSubStrings6() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See line 160 comment

}

// Tests whether method throws error when n > length
@Test

@bpsix bpsix Jun 15, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I shared a link on line 160 that talks about using Lamba expressions when dealing with exceptions in JUnit.

manipulatedstring.setString("calebrector");
int[] array = new int[] {0, 5, 1, 4, 3, 2, 6, 7, 8, 9, 10};
String restoreString = manipulatedstring.restoreString(array);
assertEquals(restoreString, "crabelector");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lol, crabelector

}

// Tests whether exceptions are thrown when an index is negative or out of bounds
@Test

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See line 160 comment. It would help to split this test case into several smaller test cases.

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