Skip to content

Cape Town | 26-ITP-Sep | Quentin Gwele | Sprint 2 | coursework/practice-TDD - #1688

Open
lwandogwele52-ui wants to merge 4 commits into
CodeYourFuture:mainfrom
lwandogwele52-ui:coursework/sprint-3-practice-tdd
Open

lwandogwele52-ui wants to merge 4 commits into
CodeYourFuture:mainfrom
lwandogwele52-ui:coursework/sprint-3-practice-tdd

Conversation

@lwandogwele52-ui

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1060

Changelist

Finished all files.

@lwandogwele52-ui lwandogwele52-ui added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 12, 2026
@lwandogwele52-ui
lwandogwele52-ui deleted the coursework/sprint-3-practice-tdd branch October 12, 2026 19:44
@lwandogwele52-ui
lwandogwele52-ui restored the coursework/sprint-3-practice-tdd branch October 17, 2026 05:16
@hackertainment hackertainment added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 19, 2026

@hackertainment hackertainment left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code generally looks good to me. Please keep it up.

Comment on lines 13 to 18
test("should count multiple occurrences of a character", () => {
const str = "aaaaa";
const char = "a";
const count = countChar(str, char);
expect(count).toEqual(5);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is a special case of str where it is all the char to count. Would it still count correctly if the char just exists in some part of the str?

Comment on lines +25 to +30
test("should return 0 when the character does not occur in the string", () => {
const str = "hello";
const char = "z";
const count = countChar(str, char);
expect(count).toEqual(0);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is a special str that would give 0 anyway. Please add that test case as well. Thank you.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your test cases are correct. Just for you to think a little bit more, besides const str = "hello", what special string should also be added to each of the test cases?

@hackertainment hackertainment added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 19, 2026
@lwandogwele52-ui lwandogwele52-ui added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Oct 19, 2026
@hackertainment

hackertainment commented Oct 20, 2026 •

Copy link
Copy Markdown

This PR has not been completed, as I didn't see test cases have been added regarding the following comments:

Can you check whether your updates has been staged and committed? Thanks a lot.

@hackertainment hackertainment removed the Complete Volunteer to add when work is complete and all review comments have been addressed. label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants