Skip to content
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

Feedback homework week 3 #1

Open
remarcmij opened this issue Sep 28, 2018 · 0 comments
Open

Feedback homework week 3 #1

remarcmij opened this issue Sep 28, 2018 · 0 comments

Comments

@remarcmij
Copy link

Hi Joudi, here is my feedback on your JS1 week 3 homework.

I can see that you had some fun with experimenting with some assignments, however there are also some things that you need to look at.

Step 4: Strings and Array challenges

  1. You are the first one to offer two rather than one solution to this. Both are fine. Most developers would probably go for the second solution, using a regular expression. We won't cover regular expressions in the lectures as it is an advanced, vast topic but at least you got a taste of it here.

  2. Looks pretty good.

    A minor point: the idea for 2.4 was that you put in words what you thought the actual values would be rather than printing the actual values:

    console.log("I think my new array will have the values: blowfish, meerkat, capricorn, giraffe, turtle")

More JavaScript

Your idea of using alert and prompt is a nice idea, however it makes running and checking your code more difficult. I have to look in two places (the web page and the console) and remember or makes notes of what I typed as input in order to check the output. So again, nice idea, but for reviewing purposes I would prefer function calls with fixed parameters.

You probably commented out the code in your initial pull request because of the alerts and the prompts hindering testing.

  1. Correct. But you do not need to use (and preferably should not use) parentheses around the value in a return statement. This holds true for all other steps in this file.

    Excellent that you use parseInt() to convert the text that the user typed in the prompt() into a number, because prompt() itself returns a string. I guess you tried it first without parseInt() and saw that something completely different happened, right?

  2. The parameter that you pass to your colorCar() function is called color but you do not use this parameter inside the function body. Instead you are grabbing the external variable myFavoriteColor as the value to use. This makes your function impure and the color parameter redundant.

  3. Your function is called displayObject() but that is not what it does: it simply returns the value of its argument without displaying anything. If you would place the console.log statement inside the function body thrn it would do at its name implies.

    The use of Object.entries is a clever find. You could also you a for loop.

  4. A good idea to make your vehicleType() functions unique by adding the assignment number to the function name. This avoids issues with defining functions with the same name in a single file.

    Excellent that you made this a pure function and use console.log outside of the function.

    You should always use the strict equality operator, i.e. the one with the triple equal signs: ===.

    It is a best practice to format an if statement in a standard way:

    if (type === 1) {
       type = 'car';
    }  else { 
      type = 'motorbike'; 
    }

    See Code Formatting in the HYF fundamentals repo.

    Note that reassigning parameters to a new value inside the function body is considered a bad practice. You are doing that here with the type parameter. First it was a number, indicating a code for a vehicle type and then it becomes a string representing a vehicle name. Instead, you could introduce a new variable, say vehicle:

    let vehicle = 'motorbike';
    if (type === 1) {
      vehicle = 'car';
    }

    Or, using the ternary operator:

    const vehicle = (type === 1) ? 'car' : 'motorbike';

    Because the assignment is done in a single statement you can use const here.

  5. Correct.

  6. The same comment from step 4 applies here too: don't reassign parameters inside the function body. Also do not change the value type that a variable holds: age was a number and now becomes a description of the car condition (new vs used). This can easily lead to confusion what a parameter represents once your code starts to grow.

  7. Correct.

  8. Correct.

  9. The assignment specifies that you should be able to call your function like this:

    vehicle("green", 3, 1)
    // console.log(vehicle("green", 3, 1)) is fine too

    Clearly your function does not meet this requirement. The code parameter should be used as an index into your vehicles array, taking into account that JavaScript array indices are zero-based.

  10. Correct. Note that the choice for 'bus' in this assignment is best avoided because the plural of 'bus' is 'buses' and not 'buss'.

  11. The idea was that you printed a new advertisement with an extra vehicle added to the array. That would have been easy if you implemented step 10 as a function that you call twice.

  12. Correct. Nice that you used const.

  13. Okay.

  14. You should use dot notation where you can when accessing object properties, in preference over bracket notation. Also, object property names should be in camelCase:

    teachers.languages = ...
  15. I think you were a bit careless with copy & paste here because all tests are about x == y. You also need to watch out for operator precedence. What does this do?

    'x==y is' + x == y ? 'equal' : 'not equal'

    Are you concatenating x to the string 'x==y is' and then comparing that with y using ==?

    Or are you comparing x and y and then concatenating 'equal' or 'unequal' to 'x==y is'?

    By adding some parentheses you can eliminate the confusion.

  16. What is your conclusion? Can you describe that in a comment?

  17. What is happening here? Can you describe that in a comment?

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

No branches or pull requests

1 participant