Skip to content

Vanessa Code Review Files#28

Open
ness-miewald wants to merge 1 commit intomainfrom
Vanessa_Miewald_CodeReview
Open

Vanessa Code Review Files#28
ness-miewald wants to merge 1 commit intomainfrom
Vanessa_Miewald_CodeReview

Conversation

@ness-miewald
Copy link

No description provided.

@sarafarag sarafarag requested a review from meelybug123 June 16, 2023 20:21

@Test
public void testGeSubStrings1() {
public void testGetSubStrings1() {

Choose a reason for hiding this comment

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

You need to add unit tests for all the if statements in getSubstrings() in StringManipulation.java
ex:

if (startWord <= 0 || endWord <= 0 || startWord > endWord) {
    throw new IllegalArgumentException("Invalid startWord or endWord values");
}
if (string == null || string.isEmpty()) {
    throw new IndexOutOfBoundsException("String is empty");
}
String[] words = string.trim().split("\\s+");
if (endWord > words.length) {
    throw new IndexOutOfBoundsException("String has fewer words than endWord");
}

@@ -55,95 +66,140 @@ public void testRemoveNthCharacter2() {

@Test
public void testRemoveNthCharacter3() {

Choose a reason for hiding this comment

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

The following test cases (testRemoveNthCharacter 3-7) are all variations of the first two test cases. Please revise these test cases so that they're testing for different kinds of input (e.g., n <= 0, string.isEmpty(), etc.)

@Test
public void testRestoreString1()
{
public void testRestoreString1() {

Choose a reason for hiding this comment

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

Need more unique test cases for testing restoreString() (e.g., string.length !== array.length, any of the indices in int[] array are negative or greater than or equal to the length of the string).

}

public String restoreString(int[] indices) {
if (string == null || indices == null || string.length() != indices.length) {

Choose a reason for hiding this comment

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

Java already throws a runtime error if you try to call restoreString() with an uninitialized int array, so I don't think you need the second condition here.

Copy link

@meelybug123 meelybug123 left a comment

Choose a reason for hiding this comment

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

Hey Vanessa, please take a look at the comments above when you get the chance!

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