Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

NW6|Bakhat Begum| Module.JS-1| Bakhat/week 4|Sprint-4|Week-4 #178

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

Conversation

BakhatBegum
Copy link

@BakhatBegum BakhatBegum commented Dec 1, 2023

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 meet the requirements of this task

Questions

I would like to receive feedback.

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.

Hey @BakhatBegum ! 👋 I took a look at them, and I have some feedback to share. While you made an effort, there are some areas where improvements can be made. I believe with a bit more attention to detail and some additional practice, you'll be able to enhance your solutions. If you have any questions or need further assistance, please don't hesitate to reach out. I'm here to help and support you in your learning journey. Keep up the good work!

// The last4Digits variable should store the last 4 digits of cardNumber
// However, the code isn't working
// Make and explain a prediction about why the code won't work
//ans: I think we use .slice method for string. According to me, .toFixed() method will work on card. The method .toFixed() is used for numerical values .

Choose a reason for hiding this comment

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

Your solution is correct. For clarification, the toFixed() method you mentioned is used to format numbers with a specific number of decimal places. You can check the mdn https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toFixed

Comment on lines 3 to 13
function countChar(str, char ){
if (/[A-Z]/.test(str)) {
return 0;
}else if (str.repeat(char) !== '') {
return str.repeat(char);
}
}
console.log(countChar('Bgft', 3))

module.exports = countChar;

Choose a reason for hiding this comment

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

The regular expression test if (/[A-Z]/.test(str)) checks for the presence of uppercase letters in the string str, which is not in line with the requirements. Instead, the function should count the occurrences of a specific character char within the string. Consider modifying this condition to check for the desired character instead of uppercase letters.
The usage of str.repeat(char) is incorrect. The repeat() method is used to repeat a string a certain number of times, but in this context, char should represent a single character to search for, not the number of repetitions. You'll need to adjust the approach to accurately count the occurrences of the specified character.
To improve the solution, I would suggest using a loop to iterate over each character in the string str. Inside the loop, compare each character with the specified character char and increment a count variable when there is a match. Finally, return the total count.
if you need any help please let me know.

@@ -0,0 +1,21 @@
function getOrdinalNumber(num){
var ordinal = +num.toString().split('').pop();

Choose a reason for hiding this comment

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

This line converts the number to a string, splits it into an array of individual digits, and then takes the last digit as the ordinal. This approach is problematic because it doesn't handle numbers greater than 9 correctly. For example, if num is 11, the resulting ordinal will be 1, which is incorrect. Also. we recommend using let or const instead of the var keyword. It is not recommended to use var.

@@ -1,4 +1,13 @@
// In this week's prep, we started implementing getOrdinalNumber

const getOrdinalNumber = require("./get-ordinal-number");

Choose a reason for hiding this comment

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

Try to implement more test cases for different scenarios

}
return true;
}
// console.log(isPrime(17));

Choose a reason for hiding this comment

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

The solution provided is great! It successfully checks if a given number is prime. To further improve the code, I would suggest using clear and descriptive variable names instead of abbreviations like "sq".

@@ -0,0 +1,30 @@
function passwordValidator(password,oldPassword){
if (password.length >= 5){

Choose a reason for hiding this comment

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

The condition if (password.length >= 5) is checking if the password length is greater than or equal to 5. However, the requirement is to check if the password has at least 5 characters.

@@ -0,0 +1,30 @@
function passwordValidator(password,oldPassword){

Choose a reason for hiding this comment

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

You should pass one argument and the function should validate it.

@@ -0,0 +1,30 @@
function passwordValidator(password,oldPassword){
if (password.length >= 5){
return false;

Choose a reason for hiding this comment

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

More importantly, as mentioned in the question, it is crucial to break down the problem into smaller steps to solve it effectively.

  • Begin by writing a function that checks if the password is longer than 5 characters.
  • Focus on passing the tests for this specific requirement before moving on to the next step.
  • Once the first step is working correctly, proceed to the second step and implement a function to validate the presence of an uppercase letter.
  • Again, ensure that this step passes the tests.
  • Repeat this process for each subsequent requirement: checking for a lowercase letter, a number, a non-alphanumeric symbol, and ensuring the password is not in the list of previous passwords.


test("When the repeat function is called with o inputs, then it should return empty string.", () => {

expect(repeat("eff", 0)).toBe();

Choose a reason for hiding this comment

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

In this test, should check ...toBe('')

case: Handle Count of 0:
// Given a target string str and a count equal to 0,
// When the repeat function is called with these inputs,
// Then it should return an empty string, ensuring that a count of 0 results in an empty output.

else if (str.repeat(count) !== '') {
return str.repeat(count);
}
return ;

Choose a reason for hiding this comment

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

As I explained below, this should return an empty string instead of undefined.

Copy link

@Sabella-8 Sabella-8 left a comment

Choose a reason for hiding this comment

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

I have seen a lot of progress, well done!

Choose a reason for hiding this comment

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

I think the task is to count the repeated characters, not simply to repeat them as you mentioned in your test description. For example: When the function is called with these inputs,Then it should correctly count consecutive occurrences of char (e.g., 'a' appears five times in 'aaaaa\

Choose a reason for hiding this comment

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

Great work!

Choose a reason for hiding this comment

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

well done!

Choose a reason for hiding this comment

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

in line 2, if the password length >= 5 should print true because we want the password length to be >=5. Same in line 11 we want the password to include special characters so it should return true.

Choose a reason for hiding this comment

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

Well done.

Choose a reason for hiding this comment

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

Good explanation

Choose a reason for hiding this comment

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

Well done!

Choose a reason for hiding this comment

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

well done

Choose a reason for hiding this comment

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

you used a lot of new codes, which is good.

Choose a reason for hiding this comment

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

good job!

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

Successfully merging this pull request may close these issues.

3 participants