Skip to content

Week 4 mm#59

Open
meriem26 wants to merge 10 commits intorarmatei:masterfrom
meriem26:week-4-MM
Open

Week 4 mm#59
meriem26 wants to merge 10 commits intorarmatei:masterfrom
meriem26:week-4-MM

Conversation

@meriem26
Copy link

@meriem26 meriem26 commented Apr 4, 2019

Week 4 exercises

Copy link

@p-maxwell p-maxwell left a comment

Choose a reason for hiding this comment

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

There's a common mistake in this set of exercises I feel is worth discussing.

Filter needs a function that just returns true or false. When it calls the function on an item in an array, if the function returns false it ignores it, and if it returns true it adds it to the new array.
You're passing functions to filter that return a string - or don't return anything at all! JavaScript will try to figure out what you mean and will turn the string into true and treat not returning anything as false, so it works, kind of, but this is hard to understand and it's very easy to end up with bugs doing this.

The function you pass to map doesn't need to do any filtering. You filter in the previous step. Usually you shouldn't have an if in the function you pass to map.

@@ -4,12 +4,11 @@
// - every resulting item run through the `capitalise` function

function tidyUpString(str) {

Choose a reason for hiding this comment

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

Try to branch from master for each new week, it makes the pull request easier. Going to skip to week 4.

// returns an array of the owners' email addresses of the two houses
function getEmailAddresses(house1, house2) {}
function getEmailAddresses(house1, house2) {
return house1.currentOwner.email + " , " + house2.currentOwner.email;

Choose a reason for hiding this comment

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

Small note, this will return a string instead of an array. It prints the right thing but if you want to index into it you can't.


var destinationNamesMoreThan300KmsAwayByTrain = travelDestinations
.filter(DestNameMoreThan300KmsByTrain)
.map(DestNameMoreThan300KmsByTrain); // Complete here (PRINT THE RESULT IN THE CONSOLE USING FOREACH)

Choose a reason for hiding this comment

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

This works, but possibly not in a way that you would expect.
travelDestinationsLessThan500Kms will return undefined if the distance is under 500. This is close enough to false for filter to work, so it filters it. You then call the same function again with map.
Similar logic applies to the others.

A better way of doing this is to have two functions for each - one that filters and one that maps e.g.

var destinationNamesWithin500Kms = 
    travelDestinations.filter(
        place => place.distanceKms < 500
    ).map(
        place => place.destinationName
    );

},
findRestaurantServingDish: function(dishName) {
function checkDish(dish) {
return restaurants.includes(dish);

Choose a reason for hiding this comment

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

Nearly - you need to check the menu though. And passing a function to restaurants.filter will make it act on each restaurant, so what you really want is more like:
function checkRestaurant(restaurant) { return restaurant.menu.includes(dishName); }.

},
countNumberOfRestaurantsInArea: function(area) {
// Complete here
function adressCount(address1, address2) {

Choose a reason for hiding this comment

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

You never use area in here, which should make you suspicious whether this works. (If you change it to west instead of center, it still returns 2).
As before you need a filter which checks if the restaurant is what you're looking for - that restaurant.address.area is the same as the area parameter - and then you don't need a map.

// calling this function should decrease your bottles volume by 10;

drink: function(volume) {
return (this.volume -= 10);

Choose a reason for hiding this comment

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

Or here.

},
empty: function() {
// this function should return true if your bottles volume = 0
// calling this function should decrease your bottles volume by 10;

Choose a reason for hiding this comment

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

careful where the comment goes, it's lying about the next function!

// calling this function should decrease your bottles volume by 10;
empty: function(volume) {
if (this.volume === 0) {
return true;

Choose a reason for hiding this comment

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

This return however is correct. :)

"."
);

writers.forEach(element => {

Choose a reason for hiding this comment

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

Good.

);
}
}
console.log(writers.filter(isAlive).map(isAlive));

Choose a reason for hiding this comment

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

Using the same function for filter and map is a clue something is wrong - look at my comment on E-arrays-of-objects, exercise 2.

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