Skip to content

Week 3 class assignment Joann Cahill #8

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jcahill53
Copy link

No description provided.


};

logReceipt();
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately by undoing the bit of boilerplate that tests the logReceipt() function we sort of lose our way as to what is being exercised. The implementation above has become brittle and not re-usable . . it's a one-use function that entirely relies upon some external state named items . . . What we're trying to get to is a function that can be passed any number of menu item objects (as the beginning file has as its test case)

Try again, and be sure that the logReceipt function can take an undetermined number of inputs . . . . remember the 'spread' operator!

}

usetopSpeed(){
const{name, topSpeed} = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

Aha! Yes, this destructuring from this is a great strategy for methods, it makes them quite readable, BUT . . . . . .


usetopSpeed(){
const{name, topSpeed} = this;
console.log( `${this.name} moving to ${this.topSpeed}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't use the destructured values, it becomes moot. You can probably see in VSCode that name and topSpeed on line 14 are kind of greyed out, or you can hove and see they are 'declared but never read' . . . . and that is because on line 15 you are accessing using this. .. . Now either way is totally valid, but it's a one-or-the-other kind of thing 😉

// console.log(parsedPhone2);

//create format to be used in return statement
let phoneNumbers = (`areaCode: ${parsedAreaCode}, phoneNumber: ${parsedPhone2} `)
Copy link
Contributor

Choose a reason for hiding this comment

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

Very near the mark, but this is an important distinction . . . the goal is to return an object with the 2 values . . . here we are returning a string! I know it's a tad trivial seeming with this exercise, but having keen intentionality of types is a powerful skill to hone.

Copy link
Contributor

Choose a reason for hiding this comment

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

refer to lines 75 and 78 for a glimpse of how the return value should look.

// console.log(parsedPhone2);

//create format to be used in return statement
let phoneNumbers = (`areaCode: ${parsedAreaCode}, phoneNumber: ${parsedPhone2} `)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also worth mentioning, this use of parenthesis around the string is not necessary

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.

2 participants