Skip to content

Add final project implementation from Assignment 2#8

Open
FLSGawsl wants to merge 1 commit intomainfrom
Chonglin_Guan_CodeReview
Open

Add final project implementation from Assignment 2#8
FLSGawsl wants to merge 1 commit intomainfrom
Chonglin_Guan_CodeReview

Conversation

@FLSGawsl
Copy link

No description provided.

Copy link

@brunochristensen brunochristensen left a comment

Choose a reason for hiding this comment

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

The code looks good and functional, I would just think about expanding the scope of your test cases.

String[] words = string.trim().split("\\s+");
int numWords = words.length;

if (startWord <= 0 || endWord <= 0 || startWord > endWord) {

Choose a reason for hiding this comment

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

This might be more of a style choice, but wouldn't it make more sense to throw exceptions before executing business logic?

char[] restoredChars = new char[chars.length];
for (int i = 0; i < indices.length; i++) {
int index = indices[i];
if (index < 0 || index >= chars.length) {

Choose a reason for hiding this comment

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

Would it make more sense to verify the integrity of your data prior to when you start executing the logic of the method?

public void testRemoveNthCharacter6() {
fail("Not yet implemented");
manipulatedString.setString("Exception testing1");
assertEquals("Don't mind this", manipulatedString.removeNthCharacter(0, true));

Choose a reason for hiding this comment

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

JUnit does provide a assertThrows() method to test exception handling. It would probably be better than relying on the exception methods returning string.

throw new IndexOutOfBoundsException("n exceeds string length");
}
StringBuilder result = new StringBuilder();
char[] chars = string.toCharArray();

Choose a reason for hiding this comment

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

It doesn't look like you ever do anything with the chars variable.


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

Choose a reason for hiding this comment

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

I would consider adding more variety or some edge cases to your testing. Maybe add some sentences with unconventional spacing for characters.

if (string == null) {
return 0;
}
String[] words = string.trim().split("\\s+");

Choose a reason for hiding this comment

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

trim() is probably an unnecessary step to separate word by whitespace.

fail("Not yet implemented");
manipulatedString.setString("Why there are so many test cases to create");
String [] sStings = manipulatedString.getSubStrings(0, 6);

Choose a reason for hiding this comment

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

How is this testing if an exception is thrown? What are the mechanics behind this?

manipulatedString.setString("art");
int [] array;
array=new int[]{2,0,1};
String restoreString = manipulatedString.restoreString(array);

Choose a reason for hiding this comment

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

What would happen if the array argument uses duplicate values?

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