Skip to content

Wesley eand code review#18

Open
birdhead9 wants to merge 3 commits intomainfrom
Wesley_Eand_CodeReview
Open

Wesley eand code review#18
birdhead9 wants to merge 3 commits intomainfrom
Wesley_Eand_CodeReview

Conversation

@birdhead9
Copy link

No description provided.

@sarafarag sarafarag requested a review from xyuovo June 16, 2023 00:57
@xyuovo
Copy link

xyuovo commented Jun 19, 2023

Hi Wesley,
In my opinion, Each test is initially set to fail with the fail("Not yet implemented"); line. This line should be removed as tests are implemented. The names of the tests are not descriptive enough to indicate what they are testing. A test name should give an indication of what functionality or aspect of functionality it is testing. You've done a good job of testing for exceptions. In particular, the tests testRemoveNthCharacter5(), testRemoveNthCharacter6(), testRemoveNthCharacter7(), testGeSubStrings3(), testGeSubStrings4(), testGeSubStrings5(), testGeSubStrings6(), testRestoreString4(), and testRestoreString5() are good examples of how to test for exception conditions. Repeated Tests: The tests testGeSubStrings5() and testGeSubStrings6() appear to be duplicates, as do testRestoreString4() and testRestoreString5(). Each test should be unique and test a different aspect of functionality. I appreciate the thoroughness of the testing. Many different cases and potential issues are checked for, which is excellent, but some of the tests should match the class implementation.

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