Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

Mickey Haile JavaScript-Core-1-Coursework-Week3 - #169

Open
mickeyhaile2 wants to merge 3 commits into
CodeYourFuture:mainfrom
mickeyhaile2:main
Open

Mickey Haile JavaScript-Core-1-Coursework-Week3#169
mickeyhaile2 wants to merge 3 commits into
CodeYourFuture:mainfrom
mickeyhaile2:main

Conversation

@mickeyhaile2

Copy link
Copy Markdown

No description provided.

@mickeyhaile2 mickeyhaile2 changed the title JavaScript-Core-1-Coursework-Week3 Mickey Haile JavaScript-Core-1-Coursework-Week3 Dec 5, 2022

@JDysiewicz JDysiewicz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There were a couple logic errors, but usually this is things like off-by-one errors which are easy to make. Overall it's looking pretty good :P

let hello = sayHello();
console.log(hello);

/* hello is a variable not a function */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In javascript, functions can be assigned to variables e.g.

const hello = sayHello
hello()

So the reason that it is undefined is because you assign hello to the result of calling sayHello(); as sayHello() doesn't return anything, hello is undefined.


sayHelloToUser();

/*needs a parameter*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

technically, it's an argument (parameter is user, argument is the value assigned to user when invoking a function)

Comment on lines +12 to +14
while (i < 2 * n){
even.push(i);}
i += 2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Think your syntax is off here; you're incrementing i after the while loop is done.

Also, for checking even numbers, the modulo % operator is what you're looking for.


// TODO - Write for loop code here

for (let i=0;i<WRITERS.length-1;i++){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

as you're starting from 0 and just using a < instead of <=, you just need to check if i<WRITERS.length instead of .length-1

// TODO
let newArr = [];
for (let title of allArticleTitles) {
if (title.length < 65) newArr.push(title);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to keep things readable, I'd advise always using {} with if statements even if they're 1 line:

if(title.length < 65) {
   newArr.push(title)
}

// TODO
for (var i = 0; i < allArticleTitles.length; i++) {
// Last i elements are already in place
for (var j = 0; j < allArticleTitles.length - i - 1; j++) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

similar to above, you don't need to -1 here

// TODO
let newArr = [];
for (let title of allArticleTitles) {
if (/[0-9]/.test(title) === true) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you could simplify this a little with a shorthand character class in regex /\d/.test)

sum += title.length;
}

return Math.round(sum / allArticleTitles.length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what if articleTitle.length is 0?

@JDysiewicz JDysiewicz added the reviewed A mentor has reviewed this code label Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

reviewed A mentor has reviewed this code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants