Glasgow | 26-ITP-Jan| Martin McLean |Sprint 3| practice TDD coursework#1025
Glasgow | 26-ITP-Jan| Martin McLean |Sprint 3| practice TDD coursework#1025mjm-git185 wants to merge 10 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
mjm-git185
added
the
Needs Review
LonMcGregor
added
the
Review in progress
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Good start on this task, there are a few things that you need to take a closer look at.
Sprint-3/2-practice-tdd/count.js
Outdated
| @@ -1,5 +1,14 @@ | |||
| function countChar(stringOfCharacters, findCharacter) { | |||
| return 5 | |||
| totalCount=stringOfCharacters.split(""); | |||
There was a problem hiding this comment.
What kind of variables are these intended to be? js lets you declare variables like this, but it is good practice to be a bit more specific.
| return "hellohellohello"; | ||
| } | ||
| function repeatStr(times, str) { | ||
| if (times < 0){ throw ("error")} |
There was a problem hiding this comment.
If you ran this program and it just said "error" would that be enough info for you to figure out and solve the problem? Could this message be more specific?
LonMcGregor
added
Reviewed
mjm-git185
added
the
Needs Review
LonMcGregor
left a comment
LonMcGregor
left a comment
There was a problem hiding this comment.
Almost done here. Just be careful about countChar - did you fully test everything after making changes? When you declare Count initially it is still capitalised, and you didn't declare it as either a const or a let
LonMcGregor
removed
the
Needs Review
|
i have now changed them all to lower case and declared the count veritable as a let |
mjm-git185
added
the
Needs Review
LonMcGregor
added
Complete
2 participants
Changelist
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
my coursework