Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NW6 | Fathi_Kahin | Module-JS1 | Week4 #176

Open
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

fhkahin
Copy link

@fhkahin fhkahin commented Nov 30, 2023

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.
This is JS1 module, Week 4 exercises (completed)

Questions

Ask any questions you have for your reviewer.

Copy link

@musayuksel musayuksel left a comment

Choose a reason for hiding this comment

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

Great job, @fhkahin ! Your week-4 solutions were excellent. Well-structured, thorough, and impressive attention to detail. If you have any questions, feel free to reach out.

// This is a function that will validate the credit card number
function isCreditCardValid(cardNumber) {
// Here we are checking if the card number is a string; if not, we are converting it to a string
let cleanedCardNumber = (typeof cardNumber === 'string') ? cardNumber.replace(/[^0-9]/g, '') : '';

Choose a reason for hiding this comment

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

To convert to sting, you can use the toString method.

cleanedCardNumber.length === 16 &&
hasAtLeastTwoDifferentDigits(cleanedCardNumber) &&
isFinalDigitEven(cleanedCardNumber) &&
isSumGreaterThan16(cleanedCardNumber)

Choose a reason for hiding this comment

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

Wow, I must say, I'm really impressed with the way you tackled the problem! The idea of breaking it down into small functions for each part is simply brilliant. Keep up the great work!

Comment on lines +42 to +44
const inputString = "aaaaaaa";
const targetChar = "a";
const currentOutput = countChar(inputString, targetChar);

Choose a reason for hiding this comment

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

You true master of readable code and & know the meaning of readable code. Well done @fhkahin

return false; // 0 and 1 are not prime numbers
}

for (let i = 2; i <= Math.sqrt(num); i++) {

Choose a reason for hiding this comment

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

This is completely true. As a further improvement, create a variable for Math.sqrt(num) to improve readability. (Like you did before)



function isPasswordValid(password, previousPasswords = []) {
if (password.length < 5) {

Choose a reason for hiding this comment

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

This task can be broken down into smaller steps by creating individual functions. You seem to be familiar with this approach :) ex:
isPasswordLongerThenFiveDigits(password) and so on so forth

//............................................
//Answer

function repeat(str, count) {

Choose a reason for hiding this comment

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

Well done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants