zydan1 / hyf-javascript3 Goto Github PK

Hi Hamza, here is my feedback on your homework.

1. I expected ratings to be an array. There can be more than one rating and your
   method getAverageRating() should compute the average of these ratings. You shouldn't use a string for a rating (like you did here: addRating('426 ( 2,100)')). If you look at the IMDB web site you can see that a rating is just a number of 'stars' than you can click (i.e a whole number between 0 and 10).
2. I also expected writers to be an array. This is hinted to by the name getWriters(). (Note the plural.)
3. In the StaffMember constructor method, the third parameter is dateOfBirth, but you are passing a year rather than a date. To pass a full date you could do this:

   new StaffMember('Akshay Kumar', 'Noh', new Date('9-9-1967'));

   and then you could compute the age:

   return currentYear - this.dateOfBirth.getFullYear();

4. To keep your StaffMember class reusable, your getAge() method should just return a number, not a string. Only where you use the age to create the innerHTML should you add the word Years. This allows you to use the same getAge() method without modification if, say, you wanted to create a Dutch version of your page and use the word jaar instead of Year.
5. addWriter() and addRating() do not return values. Therefore the values of Rating and Writers will be undefined.
6. Variable names (except class names) should start with a lowercase letter (rating, writers).
7. In the assignment it was asked to get the director's name like this: director.getName(). This hints that the director was supposed to be a StaffMember object rather than just a string.
8. Your .eslintrc.json file should be in the root folder of your repo, not in week1 or week2. And you should alway open the root folder of your repo in VSCode, not any subfolder.

I seriously do not have much to say, your html is pretty minimalist which is fine if you like it, and good job hiding some styling inside the html file rather than adding everything inside the js.

Hey Hamza,
Not to say that setAttribute is wrong by any mean but it takes lots of space you could set attributes by using dot notations for example
input.setAttribute("placeholder", "search here") => input.placeholder ="search here"; it's just for cleaning purposes.
Shame that you didn't supply a link to your html page to host it on Github but that's okay.
No comment on the code itself :)

Hi Hamza,

Here is my feedback on your finale homework.

I think your application could use improvement in the presentation of the GitHub data in your web page.

I'll now go through the individual criteria that I defined for this assignment:

1. Your code must run without errors.

   Requirement is met.

2. Your web page must be fit-for-purpose and well-designed. Place yourself in the role of an end-user: is the web page attractive and useful, or will you quickly click away?

   The data labels that you display are the keys of the GitHub JSON data you get back from a request. Those names are not user-friendly. For instance, instead of 'user_name', I would expect to see 'User Name' etc.

   The user probably also has no use for the fields id, downloads_url and subscribers_count.

   The contributor images are too big and their aspect ratio is incorrect (they are over-stretched horizontally).

   All the urls listed for the contributor are irrelevant to a user of your application. I would just display a login name and an image, and add a link to the user's GitHub page.

3. Your code must be well-formatted.

   Indentation is correct, but there are meaningless blank lines in the workOnData function.

4. Your repo must contain an .eslintrc.json file. There should be no avoidable ESLint warnings in your code. If you can fix the warning, do so. Please be prepared to account for any remaining warnings.

   Requirement is met.

5. There should be no spelling errors in variable and function names, text strings and comments. If the VSCode spelling checker flags certain words as incorrect (and sometimes that is inevitable) you should be able to explain why that is. If you are uncertain about some word, consult an English dictionary.

   Requirement is met.

6. Variable and function names must conform to these naming conventions.

   The variable Link in line 74 should start with a lowercase letter.

7. Consider blank lines to convey a meaning, similar to using blank lines to separate paragraphs in a piece of written text. A blank line between function/method definitions is fine. A blank line that breaks up a piece of code that actually belongs together is wrong. Whenever you add a blank line, carefully consider whether the blank line helps or hurts readability. Keep it in only if it helps.

   As mentioned, there are meaningless blank lines in the function workonData.

8. There should be no commented out code in your final homework. (Just like you wouldn't leave crossed out text in your CV).

   Requirement is met.

9. There should be no unnecessary console.log statements in your final code. These statements are fine when developing and debugging, but not in your final product (and, as per point 8 above, don't just comment them out).

   Requirement is met.

10. Review your own code as if you were an employer yourself. Would you offer yourself an internship or job when running and looking at the code?

    I'll let you be the judge of that.