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

Js example output value changed #1182

Merged
merged 7 commits into from Oct 26, 2018

Conversation

Projects
None yet
4 participants
@GAUTAMRAJU15
Contributor

GAUTAMRAJU15 commented Oct 13, 2018

JS example for Object.entries( ) contains wrong output value , which is being corrected

@welcome

This comment has been minimized.

welcome bot commented Oct 13, 2018

💖 Thanks for opening this pull request! 💖
Here is a list of things that will help get it across the finish line:

  • If this is a new or updated CSS interactive example, please ensure that you followed the CSS styleguide - If this is a new or updated JavaScript interactive example, please ensure that you followed the JavaScript styleguide - If your changes affects any of the steps in our contribution docs, please also make the relevant changes there.
@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 14, 2018

@GAUTAMRAJU15 Than you for your contribution. The output of the current example is correct though. As the values for the last example uses Numbers, 100 is greater than either 2 or 7 so, it end up as the last index of the Array.

If you change the example to be:

const object1 = { foo: 'bar', baz: 42 };
console.log(Object.entries(object1)[1]);
// expected output: Array ["baz", 42]

const object2 = { 0: 'a', 1: 'b', 2: 'c' };
console.log(Object.entries(object2)[2]);
// expected output: Array ["2", "c"]

const object3 = { 100: 'a', 2: 'b', 7: 'c' };
console.log(Object.entries(object3));
// expected output: Array ["2", "b"]

You will notice that the last log outputs the following:

> Array [Array ["2", "b"], Array ["7", "c"], Array ["100", "a"]]

This means console.log(Object.entries(object3))[0]; will indeed be > Array ["2", "b"] and not > Array ["100", "a"]

Reading the description section might provide some more clarity, as well as the example section.

Perhaps this needs to be explained a bit better in the documentation. Thoughts @wbamberg and @chrisdavidmills?

@wbamberg

This comment has been minimized.

Member

wbamberg commented Oct 14, 2018

Reading the description section might provide some more clarity

The description for Object.entries says: "The ordering of the properties is the same as that given by looping over the property values of the object manually" - which I think isn't very clear.

The summary, though, is clearer:" in the same order as that provided by a for...in loop". The page for for...in says: "There is no guarantee that for...in will return the indexes in any particular order. ".

So if this is true and I understand correctly, this example is not ideal. @GAUTAMRAJU15 , are you actually seeing a different result from the //expected outcome? Which browser are you using?

@GAUTAMRAJU15

This comment has been minimized.

Contributor

GAUTAMRAJU15 commented Oct 14, 2018

@schalkneethling thankyou for your guide on the issue. @wbamberg it's a little hard to understand for a new comer.
I think as @schalkneethling explained , this following code and it's description should be in mdn docs to make it easy for people to understand , what do you say?


const object3 = { 100: 'a', 2: 'b', 7: 'c' };
console.log(Object.entries(object3));

Array [Array ["2", "b"], Array ["7", "c"], Array ["100", "a"]]

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 15, 2018

@GAUTAMRAJU15 I agree with you. What if you updated the example to be as follows:

const object1 = { foo: 'bar', baz: 42 };
console.log(Object.entries(object1)[1]);
// expected output: Array ["baz", 42]

const object2 = { 0: 'a', 1: 'b', 2: 'c' };
console.log(Object.entries(object2)[2]);
// expected output: Array ["2", "c"]

const object3 = { 100: 'a', 2: 'b', 7: 'c' };
console.log(Object.entries(object3));
// expected output: Array [Array ["2", "b"], Array ["7", "c"], Array ["100", "a"]]

console.log(Object.entries(object3)[0]);
// expected output: Array ["2", "b"]

@wbamberg Thoughts?

@wbamberg

This comment has been minimized.

Member

wbamberg commented Oct 15, 2018

@wbamberg Thoughts?

As I sort of tried to say in #1182 (comment): if it's true that "there's no guarantee" that Object.entries() will return the objects in a particular order, then the example should not index into the array and expect to find a particular item. So lines 2 and 6 should also be changed.

But I would like to know what @Elchi3 thinks too :).

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 16, 2018

@wbamberg Gotcha. I have consistently been getting the expected results as seen in #1182 (comment) using both Chrome, Chrome Canary, and Firefox.

@GAUTAMRAJU15

This comment has been minimized.

Contributor

GAUTAMRAJU15 commented Oct 16, 2018

@schalkneethling okay i would update the example on this pull request as given by you for clear understanding. Thankyou

@Elchi3

This comment has been minimized.

Member

Elchi3 commented Oct 17, 2018

What Will says is correct.

JS Objects are unordered collections of properties. This is one reason why ES6 Maps were introduced. Maps iterate its elements in insertion order. (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map#Objects_and_maps_compared)

So, the order of the array returned by Object.entries has nothing to do with how you defined the object in the first place. If you want certain ordering and if you want to make sure certain elements are on certain indices, you need to sort first, like entries.sort((a, b) => a[1] - b[1]);.

I agree this should be discussed on the Object.entries page.

I think Object.entries is often used in combination with other functions and it is used when you want to deal with both key and value of the object. So, examples that just access elements at a given index might not be that useful, but some key/value manipulation with a forEach or so might be more practical.

Imo, entries is mainly about doing something with the keys and values.

Object.entries(obj).forEach(([key, value]) => {
  console.log(key + ':' + value); // or something more creative.
})

Or, alternatively:

for (let [key, value] of Object.entries(object1)) {  
  console.log(key + ':' + value);
}

But then again, without sorting things, expected output may vary.

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 24, 2018

@GAUTAMRAJU15 you wanted to update the example based on @Elchi3 comment here?
#1182 (comment)

@GAUTAMRAJU15

This comment has been minimized.

Contributor

GAUTAMRAJU15 commented Oct 26, 2018

@schalkneethling yes sir , i think the sort() function first applied and then showing the output of object.entries() would be more practical and learning assessment for people.

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 26, 2018

Thanks @GAUTAMRAJU15

@GAUTAMRAJU15

This comment has been minimized.

Contributor

GAUTAMRAJU15 commented Oct 26, 2018

btw sir , we also need to edit the description of the mdn of object.entries() page. How to do that ? i Tried to explain it there but it says , some other checks are already pending on this page ?

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 26, 2018

@GAUTAMRAJU15 thanks for the update. With regards to editing the page, generally you would just need to log-in to MDN, then an "Edit" button should be visible at the top of the page.

Clicking this will allow you to edit the content and update the page. You can also choose to ask for technical review by checking the relevant box at the bottom of the edit page. Tagging @chrisdavidmills

@schalkneethling

Couple of suggestions @GAUTAMRAJU15 - Thx!

Show resolved Hide resolved live-examples/js-examples/object/object-entries.html Outdated
Show resolved Hide resolved live-examples/js-examples/object/object-entries.html Outdated
Show resolved Hide resolved live-examples/js-examples/object/object-entries.html Outdated
@GAUTAMRAJU15

This comment has been minimized.

Contributor

GAUTAMRAJU15 commented Oct 26, 2018

Thankyou , it's done

schalkneethling and others added some commits Oct 26, 2018

Update live-examples/js-examples/object/object-entries.html
Co-Authored-By: GAUTAMRAJU15 <rajugautam45@gmail.com>
Update live-examples/js-examples/object/object-entries.html
Co-Authored-By: GAUTAMRAJU15 <rajugautam45@gmail.com>
console.log(Object.entries(result));
// expected output: Array [Array ["2", "b"], Array ["7", "c"], Array ["100", "a"]]
console.log(Object.entries(result)[0]);

This comment has been minimized.

@schalkneethling

schalkneethling Oct 26, 2018

Collaborator

I would suggest we remove lines 13-17 to shorten the overall example

@schalkneethling

This comment has been minimized.

Collaborator

schalkneethling commented Oct 26, 2018

@GAUTAMRAJU15 Thanks for the updates and sticking with the PR through its various iterations. This one is missing the comment about .sort now but, I reckon that is 100% as it is explained in the copy just preceding the example.

@schalkneethling schalkneethling merged commit bc03219 into mdn:master Oct 26, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (schalkneethling) No new issues
Details
@welcome

This comment has been minimized.

welcome bot commented Oct 26, 2018

Congrats on merging your first pull request! 🎉🎉🎉

wbamberg added a commit to wbamberg/interactive-examples that referenced this pull request Oct 29, 2018

Merge remote-tracking branch 'upstream/master'
* upstream/master:
  chore(repo) - Use consistent file naming across media assets (mdn#1191)
  chore(community) - Add @ShadowMitia as contributor (mdn#1203)
  Add "attribute-" prefix to HTML attribute examples. (mdn#1189)
  Js example output value changed  (mdn#1182)
  chore(deps): update dependency all-contributors-cli to v5.4.1 (mdn#1188)
  Issue#1192 links to media files broken (mdn#1197)
  Fix bug String.normalize live example (mdn#1193)
  chore(community) - Add @Flying-Toast as contributor (mdn#1195)
  Update html guidelines (mdn#1184)
  Use template strings to improve readability (mdn#1187)
  chore(community): add @TremaineNeethling as contributor (mdn#1183)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment