Feedback for revised homework week 2

[remarcmij]

Hi Isaac, here is my feedback on your revised homework in the folder async-promises.

I think you got my initial feedback pretty well covered and you managed to get rid of the unnecessary XMLHttpRequests. Good work!

Coming back on your sortList() function:

In your manipulateSelect() function you create a <select> element and add <option> elements in the order of the repository data returned by GitHub. Then, you use your sortList() function to correct the text and value properties of each <option> element to make them appear in alphabetically sorted order. This seems rather convoluted. Why not sort the repo array before creating <option> statements in the first place, like shown here.

function manipulateSelect(repos) {

  repos.sort((a, b) => a.name.localeCompare(b.name)); // <== added

  const select = createAndAppend('select', document.getElementById('header'));

  // ...

    repos.forEach((repo, i) => {
    createAndAppend('option', select, {
      html: repos[i].name,
      value: i
    });
  });

  // sortList(select); // <== removed

  // ...
}

So instead of sorting afterwards (and correcting what was the wrong order initially), do it directly on the data as you receive it (without creating something in the wrong order first and then correcting it). You only need a single line for the sort (but of course you need to know how you can use .sort() in combination with the localeCompare() string method).

You can now completely get rid of your sortList function.

By the way, never do this const clTexts = new Array();. Use an array literal instead: const clTexts = []. This, for the same reason that you don't use const helloWorld = new String('Hello Word'); to define a string.

[gisaac85]

ok thanks Jim i understand all your points .. but if i want to add number before every options like:

1. A
2. B
3. C ...etc

what can I do ? with my sortList function i can do this .... but now ??