Skip to content

Update skeleton#3

Open
carector wants to merge 2 commits intomainfrom
Caleb_Rector_CodeReview
Open

Update skeleton#3
carector wants to merge 2 commits intomainfrom
Caleb_Rector_CodeReview

Conversation

@carector
Copy link

Update implementation for StringManipulation and StringManipulationTest

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

bpsix commented Jun 15, 2023

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]");
Copy link

@bpsix bpsix Jun 15, 2023

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

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

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
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

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
Copy link

@bpsix bpsix Jun 15, 2023

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

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

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