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 final homework #4

Open
remarcmij opened this issue Feb 23, 2018 · 0 comments
Open

Feedback final homework #4

remarcmij opened this issue Feb 23, 2018 · 0 comments

Comments

@remarcmij
Copy link

Hi Fadi, here is my feedback on your final homework.

Overall, your homework looks good. But there are some issues in the code that suggest that this is not final code.

  • Your function hyfRep uses createAndAppend and a forEach but your function hyfUser seems to be older, and doesn't use the aforementioned. This is inconsistent.
  • Line 104: for (const key in data): data is an array. You must only use a for-in loop with objects. You can use for (const repo of data) instead.

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?

    Requirement is met, although personally, I prefer a more boring, to the point presentation style.

  3. Your code must be well-formatted.

    Requirement is met.

  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.

    Requirement is met.

  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.

    At several places there are meaningless blank lines.

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

    The commented out code in line 101 does not belong in final code.

  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.

I've refactored your code (see below) to consistently use an (enhanced) version of createAndAppend. I also made changes to some variable names and corrected the errors that I listed above. Finally, rather than using global variables, I've used function parameters.

"use strict";

{

  function start() {
    const input = document.getElementById('input');
    const repoContainer = document.getElementById('rep');
    const contributorContainer = document.getElementById('users');

    const clearContainers = () => {
      contributorContainer.innerHTML = "";
      repoContainer.innerHTML = "";
    };

    document
      .getElementById('rep_Button')
      .addEventListener('click', () => {
        clearContainers();
        onSearchRepoClick(repoContainer, contributorContainer, input.value);
      });
    document
      .getElementById('user_Button')
      .addEventListener('click', () => {
        clearContainers();
        onSearchUserClick(repoContainer, input.value);
      });
  }

  function fetchJSON(url) {
    return new Promise((resolve, reject) => {
      const xhr = new XMLHttpRequest();
      xhr.open('GET', url);
      xhr.responseType = 'json';
      xhr.onload = () => resolve(xhr.response);
      xhr.onerror = () => reject(xhr.statusText);
      xhr.send();
    });
  }

  function onSearchRepoClick(repoContainer, contributorContainer, repoName) {
    const url = 'https://api.github.com/repos/HackYourFuture/' + repoName;
    fetchJSON(url)
      .then(data => {
        if (data.message) {
          throw new Error(data.message);
        }

        createAndAppend('h1', repoContainer, 'Repository Name : ' + data.name);
        createAndAppend('h2', repoContainer, 'Description : ' + data.description);
        createAndAppend('h3', repoContainer, 'Created at : ' + formatDate(data.created_at));
        createAndAppend('h3', repoContainer, 'Updated at ' + formatDate(data.updated_at));
        createAndAppend('h3', repoContainer, 'Pushed at : ' + formatDate(data.pushed_at));
        createAndAppend('h3', repoContainer, 'Forks : ' + data.forks);
        createAndAppend('h3', repoContainer, 'On Branch : ' + data.default_branch);

        console.log(data);
        const hyfRepLink = `<a href="${data.html_url}" target="_blank">View ${data.name}</a>`;
        createAndAppend('h3', repoContainer, hyfRepLink);
        return fetchJSON(data.contributors_url);
      })
      .then(contributors => {
        createAndAppend('h2', contributorContainer, 'Contributors : ');
        contributors.forEach(contributor => {
          createAndAppend('h2', contributorContainer, 'Name : ' + contributor.login);
          const contributorImgLink = `<img src="${contributor.avatar_url}">`;
          createAndAppend('div', contributorContainer, contributorImgLink);
          createAndAppend('h2', contributorContainer, 'contributions : ' + contributor.contributions);
        });
      })
      .catch(error => {
        createAndAppend('h1', repoContainer, error.message);
      });
  }

  function formatDate(dateString) {
    return new Date(dateString).toLocaleString();
  }

  function onSearchUserClick(repoContainer, userName) {
    const url = "https://api.github.com/users/" + userName + "/repos";
    fetchJSON(url)
      .then(data => {
        if (data.message) {
          throw new Error(data.message);
        }

        createAndAppend('h1', repoContainer, data[0].owner.login);
        createAndAppend('img', repoContainer, null, {
          src: data[0].owner.avatar_url
        });
        createAndAppend('a', repoContainer, `<h3>Visit user</h3>`, {
          href: data[0].owner.html_url,
          target: '_blank'
        });
        createAndAppend('h1', repoContainer, 'Repositories :');

        data.forEach(repo => {
          createAndAppend('h2', repoContainer, 'Repository name : ' + repo.name);
          createAndAppend('h3', repoContainer, 'Description : ' + repo.description);
          createAndAppend('h3', repoContainer, 'Created at : ' + formatDate(repo.created_at));
          createAndAppend('h3', repoContainer, 'Updated at : ' + formatDate(repo.updated_at));
        });
      })
      .catch(error => {
        createAndAppend('h1', repoContainer, error.message);
      });
  }

  function createAndAppend(name, parent, innerHTML, attributes) {
    attributes = attributes || {};
    const child = document.createElement(name);
    parent.appendChild(child);
    if (innerHTML != null) {
      child.innerHTML = innerHTML;
    }
    Object.keys(attributes).forEach(name => {
      child.setAttribute(name, attributes[name]);
    });
    return child;
  }

  start();
}
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