Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions Sprint-3/3-stretch/creditCard-validator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
function creditCardValidator(cardNumber) {
// Remove all - and spaces from the input
const sanitized = cardNumber.replace(/[-\s]/g, "");

//check if the length of the sanitized input is 16
if (sanitized.length !== 16) {
return false;
}
// check if the sanitized input contains only digits
if (!/^\d{16}$/.test(sanitized)) {
Copy link
Member

Choose a reason for hiding this comment

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

By putting a {16} here, you're checking the length again. This means we're checking the length twice.

This is fine, but if we wanted to change the expected length of a credit card, we'd need to change both.

Can you think how to avoid this duplication?

Copy link
Author

Choose a reason for hiding this comment

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

Hi Daniel,
Thank you for pointing out that I have repeated the code. Regarding the issue, I have added a variable to store the value 16, and learned that if I want to apply the variable in a pattern, I need to call a new Regex function with the variable. It is something new and useful for me.

Copy link
Member

Choose a reason for hiding this comment

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

This works! There are a couple of other ways I could imagine going about this - can you think what they may be? (Happy to just discuss them in this comment thread, you don't need to change the code any more :))

Copy link
Author

Choose a reason for hiding this comment

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

I guess I understood what you meant by 'other ways.' Since we only have to check the length of 16 for once, the function will fail at the first condition check if the card number is not 16. I don't have to do it again in the second condition check. I could have focused on the characters that are all digits like /^\d+$/

Copy link
Member

Choose a reason for hiding this comment

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

Exactly :) Alternatively you could keep the {16} in the regex and delete the first length check completely (because any non-16-character string would fail the regex check).

return false;
}

// check if there ara at least two different digits
const uniqueDigits = new Set(sanitized);
if (uniqueDigits.size < 2) {
return false;
}

// check if the last digit is even
const lastDigit = Number(sanitized[sanitized.length - 1]);
if (lastDigit % 2 !== 0) {
return false;
}

// check if the sum of all digits is greater than 16
const sum = sanitized
.split("")
.reduce((total, digit) => total + Number(digit), 0);
if (sum <= 16) {
return false;
}

return true;
}

module.exports = creditCardValidator;
29 changes: 29 additions & 0 deletions Sprint-3/3-stretch/creditCard-validator.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const creditCardValidator = require("./creditCard-validator");

describe("creditCardValidator", () => {
test("should return true for a 16 digit long card number", () => {
expect(creditCardValidator("1234-5678-9012-3456")).toBe(true);
expect(creditCardValidator("1234 5678 9012 3456")).toBe(true);
Copy link
Member

Choose a reason for hiding this comment

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

Your tests all currently assume there are extra characters in the credit card number - is 123456789012345 valid? You may want to test that kind of thing too, to make sure your implementation doesn't assume there are separators.

Copy link
Author

Choose a reason for hiding this comment

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

You are right. I should have included tests for card numbers that are more or fewer than 16 digits. As you suggested, I created one test for the case of fewer and another for more.

Copy link
Member

Choose a reason for hiding this comment

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

These extra tests look good, but aren't quite getting at the problem I was pointing at - can you add a test for a 16-digit credit card number which is only numbers (i.e. no spaces or -s)?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah. I included a test for only 16 digits without spaces or operators.

});

test("should return false if the card number is not digit only", () => {
expect(creditCardValidator("1234-5678-9012-345a")).toBe(false);
expect(creditCardValidator("1234 5678 9012 3456")).toBe(true);
});

test("should return false if the card number does not contain at least two different digits", () => {
expect(creditCardValidator("1111-1111-1111-1111")).toBe(false);
expect(creditCardValidator("1111 2222 2222 2222")).toBe(true);
});

test("should return false if the card number does not end with an even digit", () => {
expect(creditCardValidator("1234-5678-9012-3457")).toBe(false);
expect(creditCardValidator("1234 5678 9012 3456")).toBe(true);
});

test("should return false if the sum of all the digits is not greater than 16", () => {
expect(creditCardValidator("0000-0000-0000-0000")).toBe(false);
expect(creditCardValidator("1111 1111 1111 1111")).toBe(false);
expect(creditCardValidator("1234 5678 9012 3456")).toBe(true);
});
});
13 changes: 13 additions & 0 deletions Sprint-3/3-stretch/find.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ console.log(find("code your future", "z"));
// Pay particular attention to the following:

// a) How the index variable updates during the call to find
// The index variable is set to 0 on line 2.
// When the function is called, the while loop on line 4 checks if index (0) is less than the length of the string (16).
// If the character is not found, the index will increment by 1 on line 8
// Now, index is 1, and the while loop will check again, until the character is found or the index is no longer less than the length of the string.

// b) What is the if statement used to check
// The if statement on line 5 checks if the character at current index of the string is equal to the target character (char).
// If they are equal, the function returns the current index.

// c) Why is index++ being used?
// The index++ on line 8 is being used to update the index variable by incrementing it by 1.
// Given the character is not found at the current index, we need to move to the next index.

// d) What is the condition index < str.length used for?
// The condition index < str.length on line 4 is used to ensure that the while loop continues to execute as long as the index is less than the length of the string.
// More importantly, it prevents the while loop goes on forever into an infinite loop.
41 changes: 38 additions & 3 deletions Sprint-3/3-stretch/password-validator.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,41 @@
function passwordValidator(password) {
return password.length < 5 ? false : true
}
const lengthCondition = password.length >= 5;
console.log(`length condition: ${lengthCondition}`);

const uppercaseCondition = /[A-Z]/.test(password);
console.log(`upper case condition: ${uppercaseCondition}`);

const lowercaseCondition = /[a-z]/.test(password);
console.log(`lower case condition: ${lowercaseCondition}`);

const numberCondition = /[0-9]/.test(password);
console.log(`number condition: ${numberCondition}`);

const symbolCondition = /[!#$%.*&]/.test(password);
console.log(`symbol condition: ${symbolCondition}`);

const previousPasswords = [
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it should probably be input to the function, rather than something we hard-code in the function itself?

Copy link
Author

Choose a reason for hiding this comment

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

Agree. I felt that it was something wrong to put a list of previous passwords inside a function. It should have been an input, which can be an array from a database or something very long and editable.

"123Ab!",
"abcD1#",
"Password1!",
"Qwerty1*",
"Zxcvbnm2$",
];
const notInPreviousPasswords = !previousPasswords.includes(password);
console.log(`previous password condition: ${notInPreviousPasswords}`);

if (
lengthCondition &&
uppercaseCondition &&
lowercaseCondition &&
numberCondition &&
symbolCondition &&
notInPreviousPasswords
) {
return true;
} else {
return false;
}
}

module.exports = passwordValidator;
module.exports = passwordValidator;
37 changes: 28 additions & 9 deletions Sprint-3/3-stretch/password-validator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,31 @@ To be valid, a password must:
You must breakdown this problem in order to solve it. Find one test case first and get that working
*/
const isValidPassword = require("./password-validator");
test("password has at least 5 characters", () => {
// Arrange
const password = "12345";
// Act
const result = isValidPassword(password);
// Assert
expect(result).toEqual(true);
}
);

describe("passwordValidator", () => {
const password = "123Ab*";

test("returns true for passwords with at least 5 characters", () => {
expect(isValidPassword(password)).toBe(true);
});

test("return true for passwords with at least one uppercase letter", () => {
expect(isValidPassword(password)).toBe(true);
});

test("return true for passwords with at least one lowercase letter", () => {
expect(isValidPassword(password)).toBe(true);
});

test("return true for passwords with at least one number", () => {
expect(isValidPassword(password)).toBe(true);
});

test("return true for passwords with at least one non-alphanumeric symbol", () => {
expect(isValidPassword(password)).toBe(true);
});

test("return true for passwords that are not in the previous passwords array", () => {
expect(isValidPassword(password)).toBe(true);
});
});