Skip to content

Week 4 & week-4-practice by Vicky#62

Open
viktoriia-kosenko wants to merge 8 commits intorarmatei:masterfrom
viktoriia-kosenko:Week-4
Open

Week 4 & week-4-practice by Vicky#62
viktoriia-kosenko wants to merge 8 commits intorarmatei:masterfrom
viktoriia-kosenko:Week-4

Conversation

@viktoriia-kosenko
Copy link

No description provided.

Copy link
Collaborator

@tekul tekul left a comment

Choose a reason for hiding this comment

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

👍 Great work. I had one or two small suggestions and questions so try to get back to me on those. No major problems with objects though 🙂 .

)
);

messageAboutWriter;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good! Why the messageAboutWriter on the last line though?

Copy link
Author

Choose a reason for hiding this comment

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

Have no idea why. But I've already deleted it.

) {
return "Please take your " + coffee;
} else {
return "Sorry you don't have enough money for a " + coffee;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would just use a plain string in each case, rather than appending the variable coffee, so "Please take your cappuccino", since you know that's what it is. However this code is quite repetitive (it has return "Please take your " + coffee; 3 times). It might be better to separate the lookup of the price from the printing out of the "Please take your ..." messages. You could have something like this:

  getCoffee: function(coffee) {
    var price = this.prices[coffee];

    if (this.insertedAmount >= price) {
      return "Please take your " + coffee;
    }   
    return "Sorry you don't have enough money for a " + coffee;
  }

Copy link
Author

Choose a reason for hiding this comment

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

your solution looks perfect!

volume: 0,
fill: function() {
// calling this function should make you bottles volume = 100;
return (this.volume = 100); // calling this function should make you bottles volume = 100;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the return here (and in the drink function)?

var personsYoungerThan28YearsOld = // Complete here
var personNames = persons.map(person => person.name); // Complete here

var personsYoungerThan28YearsOld = persons.filter(person => person.age < 28); // Complete here
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

)
.map(destination => destination.destinationName); // Complete here (PRINT THE RESULT IN THE CONSOLE USING FOREACH)

destinationNamesMoreThan300KmsAwayByTrain;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the variable name here again? Are you confusing this with calling functions? You don't need to do this with variables.

Copy link
Author

Choose a reason for hiding this comment

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

to be honest i don't know why it is there. But maybe I wasn't attentive enough and i didn't notice it. I deleted it.

0
)
.map(restaurant => restaurant.name);
// Complete here
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try to get in the habit of removing any outdated comments when you commit. You could remove the // Complete here comments when you've done the work.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I will

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