Repository navigation
Conversation
…ing the task again and see I must not use .repeat.... here we go again then ^_^
hackertainment
left a comment
There was a problem hiding this comment.
Thank you for your coursework. Please keep it up.
There was a problem hiding this comment.
Your code is correct. Just wondering what is the advantage of using this approach over other implementations? Thank you.
There was a problem hiding this comment.
Of course from what I understand, its when its more test driven it helps you build the code as your making tests for it, it helps keep a better flow and streamlined work on the code while addressing the tests at the same time. If you just build the code itself without tests in this way it can lead to unseen issues that end up rebuild large chunks of the code that can result in making more bugs and issues.
There was a problem hiding this comment.
Sorry maybe my original question was not clear. I didn't mean the advantages of TDD, I was wondering how would you come up with the following approach with the given test cases:
charMatch = str.match(new RegExp(char, "g"))charMatch === nullreturn charMatch.length
Especially what is the advantage of such implementation but not other programming logics? Thank you for your claification.
There was a problem hiding this comment.
I think it was when I was building up the code, I looked for something to use to count the "char" but i cant recall which i used but was only counting the first letter so when I searched for options on MDN i found .match() and used that one.
I found out about RegExp on the same page as .match() and saw it was used to help with finding the pattern of what i was searching for.
I still need to look into it more to understand it better to be honest.
If i think of it, this takes all the occurrences of g out of the string
For the null one I ran into an error that was showing null instead of 0 at the time so i think I put in an if to catch it out to show 0.
After finding those when I was stumped I couldn't think of any other logics I could have used at the time. I think i did it diffrently before so will look it up after this.
There was a problem hiding this comment.
No worries, although the explanations are not quite accurate, at least I understand your thinking. Even this is not the best implementation, I would let you pass this one because the program runs correctly. Maybe try to book a mentored coding session with volunteer to try another way to solve the same exercise. Thank you.
p.s.: the g in new RegExp(char, "g") means "global search" but not searching for character "g".
| test("should count multiple occurrences of a character", () => { | ||
| const str = "aaaaa"; | ||
| const char = "a"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(5); | ||
| }); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
yes, I forgot to do a test for that and have now put one in for that test of a random letters in a string.
| test("should count 0 with no occurrences of a character", () => { | ||
| const str = "aaaaa"; | ||
| const char = "b"; | ||
| const count = countChar(str, char); | ||
| expect(count).toEqual(0); | ||
| }); |
There was a problem hiding this comment.
There is a special str that would give 0 anyway. Please add that test case as well. Thank you.
There was a problem hiding this comment.
Is that when str has nothing in it? I have added a test for this aswell as some others that have just thought of.
| if (11 <= Number(numString.slice(-2)) && 19 >= Number(numString.slice(-2))) { | ||
| return numString + "th"; | ||
| } else if ( | ||
| -11 >= Number(numString.slice(-2)) && | ||
| -19 <= Number(numString.slice(-2)) | ||
| ) { |
There was a problem hiding this comment.
Please be aware of the consistency of code formatting - while the first if using one line for the conditions but the second if using multiple lines for the conditions. I personally would prefer one line which may usually be easier to read, unless explicitly specified by coding standard in a business environment.
There was a problem hiding this comment.
I am aware of that, this is the doing of the auto format that we had to put into vscode. I tried to keep it like line 3 and it just automatically does that format, when I saw it doing that I tried to make it then be like lines 6 and 7 but then it forces it back to how it was at 3. Not sure how to stop it doing that when I hit tell it to save.
There was a problem hiding this comment.
Thanks for letting me know. I will discuss with cirriculum team to improve the auto format file. All good for now.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Ah i think I understand, ones where it has a space or symbols, even an empty one. Added those tests.
There was a problem hiding this comment.
Yes, sometimes space or symbols can be boundary cases. In the context of this program, only empty and non-empty string matters. Empty string with any positive or zero count should still be empty. However, empty string with negative count should still throw error. Some trainees also return empty string for negative count which would not be quite accurate "for the context of this question". This is why we need to test empty string "" together with "hello"in each of your original test suites - not adding empty string as a separate test suite. Since your program is correct, I will let you pass this time (even testing of empty string for count = -3 is missing) but understand the design process should be your focus. You may also book a mentored coding session with a volunteer to learn more about test cases design. Thank you.
There was a problem hiding this comment.
Ah didn't think of the negatives there, need to remeber those. I see what you mean, I'll look into the mentored coding session I got coming up to ask about it but will look into it aswell.
|
Just remain this new comment #1697 (comment) , thank you. |
|
Congratulation, you have completed this PR successfully. |
Learners, PR Template
Self checklist
Task code
CYF-1060
Changelist
Completed the tasks by doing tdd to the best of my ability.