Feature/week 4 exercises #203

SyedArslanHaider · 2024-11-03T15:25:17Z

Subject: Pull Request Review Request

Hi Sergey,

I hope you are doing well. I have submitted my pull request for review. I would appreciate it if you could take a look at it when you have the time.

I want to mention that I tried to solve the problem by myself first, and I did some research online for additional insights. This practice has been really helpful for me in deepening my understanding.

Thank you for your guidance and support!

nsi88 · 2024-11-04T08:06:59Z

week-4/implement/credit-card-vslidator.js

@@ -0,0 +1,26 @@
+function creditCard(value){
+    const cardPattern = /^\d{16}$/.test(value);


Where did you take the rules from?
I mean 16 digits, at least 2 different digits, etc.
From what I see in Wikipedia the rules are different https://en.wikipedia.org/wiki/Payment_card_number.
Do you want to try to implement the rules? It would be an amazing excercice. Especially the the Luhn algorithm to check the last number.

I tried implementing a credit card feature, but my initial solution didn’t fully meet real-world requirements. I realized how much detail and accuracy are needed to make it work correctly. Even though my first attempt wasn’t perfect, I find it amazing to learn what’s involved in a proper implementation, and I’m excited to keep improving it to align with true real-world standards like implementing the Luhn algorithm to check the last character

nsi88 · 2024-11-04T08:10:52Z

week-4/implement/credit-card-vslidator.js

@@ -0,0 +1,26 @@
+function creditCard(value){


It may be too much at once. And it's not a requirement for the task. But it would be good to pay attention already to function naming and interface.
What does your function do? The current name doesn't tell that. Something about credit cards.
What should the function return? Just printing is not that useful in production. You will need to call the function from another function.
Here are a couple of examples I see could be clearer.

validateCredictCard. The function would raise an exception is the card number is invalid or would just finish not returning anything if all good.

isCreditCardValid. The function would return either true or false depending if the number is valid or not.

Yes I understand but they give these instructions for credit card implementation

In this project you'll write a script that validates whether or not a credit card number is valid.

Here are the rules for a valid number:

Number must be 16 digits, all of them must be numbers.

You must have at least two different digits represented (all of the digits cannot be the same).

The final digit must be even.

The sum of all the digits must be greater than 16.

For example, the following credit card numbers are valid:

9999777788880000 6666666666661666

And the following credit card numbers are invalid:

a92332119c011112 (invalid characters) 4444444444444444 (only one type of number) 1111111111111110 (sum less than 16) 6666666666666661 (odd final number) It's a simple implementation, but I will try to improve it according to your suggestions. I’m truly happy to have your guidance as I work on improving my skills, and I appreciate the time you take to help me understand how to make my code better.

Ah. Makes sense then. I didn't know you had precise requirements. All good then.
Did you have any questions about the function?

nsi88 · 2024-11-04T08:15:22Z

week-4/implement/get-ordinal-number.test.js

+    const sufix = (value > 10 && value <20) ? suffixes[0] : suffixes[Math.min(value%10,3)];
+    return value+sufix;
+}
+console.log(getOrdinal(1));   // "1st"


You need more tests. Try 5 for example)

Also the tests have to be written using Jest.
Here no better advice than just reading their docs.
Not an easy read but an important one. I think the biggest amount of time is spent in either writing tests or debugging. Not in writing code. So you should be very proficient and comfortable in tests.

"Yes my code isn’t working properly, but I’ll definitely read through the document you shared. If I run into any confusion, I’ll reach out to you for help. Thanks for your guidance!"

nsi88 · 2024-11-04T08:22:26Z

week-4/implement/is-prime.test.js

@@ -1,3 +1,26 @@
 // Given a positive integer num,
 // When the isPrime function is called with num as input,
 // Then it should return a boolean representing whether the num is prime
+function isPrime(num){


Nice. How did you come up with this solution?

If conditions are written by my own and I search Google for loop code but this is very helpful

nsi88

Looks good. You learned jest really fast.
Any questions? Anything doesn't work?

nsi88 · 2024-11-08T10:58:45Z

week-4/implement/count.test.js

-console.log(countChar("banana", "a")); // Expected output: 3
+describe('countChar',()=>{
+  test('should return correct count for a specific character',()=>{
+    expect(countChar('aaaaa','a')).toBe(5);


The test looks good. Nice.
For future you can use test.each. Shorter this way and allows to add more tests easily.

ok I will search about test.each

nsi88 · 2024-11-08T11:01:19Z

week-4/implement/credit-card-vslidator.test.js

+// creditCard.test.js
+const creditCard = require('./credit-card-vslidator');
+
+describe('creditCard function', () => {


yes It works good I learned how test is worked mostly tests are passed but I think 2 test in password validator and 2 in card validator are failed I think the logic of the code has an error that's why some tests looks failed

Which tests failed?

nsi88 · 2024-11-08T11:03:36Z

week-4/implement/get-ordinal-number.test.js

+describe('getOrdinal', () => {
+  test('returns "1st" for 1', () => {


test.each would be very handy in here.
Would be good not to miss numbers before 10 at least.

Yes I try to learn test.each also if feels any difficulty I will let you know

nsi88 · 2024-11-08T11:07:03Z

week-4/implement/credit-card-vslidator.js

@@ -0,0 +1,26 @@
+function creditCard(value){


Ah. Makes sense then. I didn't know you had precise requirements. All good then.
Did you have any questions about the function?

SyedArslanHaider · 2024-11-08T16:24:28Z

Credit card validator but I fix this now some test of the password validator tests should fails

…

On Fri, 8 Nov 2024, 4:35 pm Sergey Novikov, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In week-4/implement/credit-card-vslidator.test.js <#203 (comment)> : > @@ -0,0 +1,30 @@ +// creditCard.test.js +const creditCard = require('./credit-card-vslidator'); + +describe('creditCard function', () => { Which tests failed? — Reply to this email directly, view it on GitHub <#203 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHWOUANEENNKZFNEIEMOHVLZ7TK5VAVCNFSM6AAAAABRC2RDDKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMRUGEZDCNRYGI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

nsi88 · 2024-11-09T06:48:11Z

Credit card validator but I fix this now some test of the password validator tests should fails

Ok. Happy to help with the failing test if needed. But I need to know which one is failing.

SyedArslanHaider · 2024-11-09T09:27:15Z

Hi Sergey,

I ran Jest tests on the password validator, but I'm encountering a few issues. Here’s a summary of the errors:

Undefined Variable:
In multiple tests, previousPasswords is throwing a ReferenceError. Specifically, in lines like:

else if(previousPasswords.includes(pass)){
It looks like previousPasswords hasn't been defined or passed correctly to the validPassword function.
Mismatch in Expected vs. Received Error Message:

In the test case should return error if password does not contain at least one number, Jest expected this error message:

"Error: Password must contain at least one number(0-9)"
but instead received:

"Error: Password must contain at least one lowercase letter (a-z)"
This suggests there might be an error in the logic handling the number requirement.
Could you take a look with me? I’d appreciate any insights on handling the undefined variable or adjusting the logic for the number requirement error. Thanks!

nsi88 · 2024-11-12T07:26:00Z

Hey hey,

First about the undefined variable.
A popular problem and not an easy concept.
If shortly variables are visible if they are defined in the same function (including if passed as parameters) or in a parent function/context.
An example:

let global_variable = 1;
function parent() {
  let parent_variable = 2;
  function child(param) {
    let child_variable = 3;
  }
}

In the example function child will see all the variables.
Function parent will see parent_variable and global variable.

nsi88 · 2024-11-12T07:28:43Z

Next an example when a function won't see a variable:

function1() {
  let variable1 = 1;
}
function2() {
  let variable2 = 2;
}

Here function2 is not defined within function1 and can't see its variables.
This is example related to your test.
You define previousPasswords in your test function describe('validPassword',()=>{
But you're trying to access it in a different function validPassword

nsi88 · 2024-11-12T07:30:16Z

There are couple ways to make this work:

Define the previousPasswords variable in the common context. For example on the top of the file. Then both validPassword and its test will see it.
Pass previousPasswords as a parameter to validPassword.

nsi88 · 2024-11-12T07:31:45Z

Try this article about variables scope in Javascript https://www.w3schools.com/js/js_scope.asp
Or the book on Javascript you have should describe scopes as well

nsi88 · 2024-11-12T07:32:07Z

Please ask what's not clear.
Happy to discuss further on a call if you need.

nsi88 · 2024-11-12T07:35:08Z

"Error: Password must contain at least one number(0-9)" but instead received:

"Error: Password must contain at least one lowercase letter (a-z)"

See how your function works? It first checks lowercase. If all good, then it checks numbers.
The problem is with the test. The test password needs to have only one error at a time to have a pure test.
Does it make sense?

SyedArslanHaider added 2 commits November 3, 2024 13:17

write code of 16 digit card validator

41897dc

complete the implimentation part and also investigate part of module-js4

b414a81

nsi88 reviewed Nov 4, 2024

View reviewed changes

performe jest testing in all files

c22c0d5

nsi88 reviewed Nov 8, 2024

View reviewed changes

UmairRiazDev mentioned this pull request Nov 9, 2024

[TECH ED] Code review UmairRiazDev/Coursework-Planner#149

Closed

		@@ -0,0 +1,26 @@
		function creditCard(value){
		const cardPattern = /^\d{16}$/.test(value);

		describe('getOrdinal', () => {
		test('returns "1st" for 1', () => {

Uh oh!

Feature/week 4 exercises #203

Are you sure you want to change the base?

Feature/week 4 exercises #203

Uh oh!

Conversation

SyedArslanHaider commented Nov 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SyedArslanHaider Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nsi88 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SyedArslanHaider commented Nov 8, 2024 via email

Uh oh!

nsi88 commented Nov 9, 2024

Uh oh!

SyedArslanHaider commented Nov 9, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

nsi88 commented Nov 12, 2024

Uh oh!

Uh oh!

SyedArslanHaider Nov 4, 2024 •

edited

Loading