Skip to content

Added implementation for StringManipulation Files#14

Open
NadavH12 wants to merge 2 commits intoSWE-CS410:mainfrom
NadavH12:main
Open

Added implementation for StringManipulation Files#14
NadavH12 wants to merge 2 commits intoSWE-CS410:mainfrom
NadavH12:main

Conversation

@NadavH12
Copy link

No description provided.

return words.length;
}


Choose a reason for hiding this comment

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

This code shows good practice in terms of readability, by using specific and detailed comments before each method header in the class.

if (string == null || string.length() == 0)
return 0;
//trim leading and ending whitespace
String copy = this.getString();

Choose a reason for hiding this comment

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

Since the string is already a member of the class, there's no need to call a getter method of getString() to access it, nor is there any reason to copy the string every time -- none of these methods are mutators.

Copy link
Author

Choose a reason for hiding this comment

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

I make a copy here because afterwards I call the String.trim() method, which removes leading and lagging whitespace. Just for the sake of not changing the original string.

@@ -1,116 +1,197 @@
/* Nadav Horowitz 6/3/2023 StringManipulationTest.java
Copy link

@ElCapitanHaddock ElCapitanHaddock Jun 17, 2023

Choose a reason for hiding this comment

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

Test cases meaningfully cover almost all bounds and thrown exceptions, without any padded or trivial test cases.

assertEquals(sStings[1], "string");
}

@Test

Choose a reason for hiding this comment

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

This whitespace test might have been better suited by a lower out-of-bounds exception test for a negative start word parameter.

Copy link

@ElCapitanHaddock ElCapitanHaddock left a comment

Choose a reason for hiding this comment

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

Change the getSubString whitespace test case to one checking the lower bound.

@NadavH12
Copy link
Author

Changed the getSubString whitespace test case to one checking the lower bound.

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