Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Misleading console output for arrays with undefined #1651

Closed
Kos opened this issue Sep 4, 2011 · 23 comments
Closed

Misleading console output for arrays with undefined #1651

Kos opened this issue Sep 4, 2011 · 23 comments

Comments

@Kos
Copy link

Kos commented Sep 4, 2011

Code:

var list1 = [1, undefined, 3]; var list2 = [1, 2, 3] delete list2[1] console.log(list1) console.log(list2) console.log(list1[1]) console.log(list2[1])

Output current:

[ 1, undefined, 3 ] [ 1, 3 ] undefined undefined

Output expected:

[ 1, undefined, 3 ] [ 1, undefined, 3 ] undefined undefined

@mmalecki
Copy link

mmalecki commented Sep 4, 2011

Your assumption isn't really valid.

Perhaps this kind of array should be printed like a dictionary, but it can be misleading as well.

@bnoordhuis
Copy link
Member

> a = [1,2,3]; delete a[1]; a.toString()
'1,,3'

What do you prefer?

@jimschubert
Copy link

I can see why the output would be misleading.

[ 1, 3 ] suggests that the index of 3 is 1, but arr[1] is undefined. arr[2] is 3 even after deleting a preceding value.

@NuckChorris
Copy link

And this is why we don't use delete for arrays, we us Array.slice()

@medikoo
Copy link

medikoo commented Sep 5, 2011

deleting items from array lead to misleading results ;-)

@NuckChorris
Copy link

That's because deleting is improper on an array. :P

Sent from my shiny glass rectangle

On Sep 5, 2011, at 12:43 AM, [email protected] wrote:

deleting items from array lead to misleading results ;-)

Reply to this email directly or view it on GitHub:
#1651 (comment)

@mmalecki
Copy link

mmalecki commented Sep 5, 2011

True, but there should be some way of printing this kinds of arrays.
I'm +1 on @bnoordhuis' solution. We could make it look like [ 1, , 3 ] after util.format.

@medikoo
Copy link

medikoo commented Sep 5, 2011

@mmalecki this would be misleading as well, I would say it'll be best to display it with all properties named then:
[ '0': 1, '2': 3 ]

@mmalecki
Copy link

mmalecki commented Sep 5, 2011

@medikoo These are not string indexes, so it should be: [ 0: 1, 2: 3 ], I think. But yes, it looks good (pretty hard to mislead with dictionary).
I will try to provide patch for this one.

@NuckChorris
Copy link

I think a dictionary misleads people into thinking it's a dictionary.

@NuckChorris
Copy link

Also +1 on @bnoordhuis solution

@Kos
Copy link
Author

Kos commented Sep 6, 2011

In both cases I've originally mentioned, it holds that:

  • list.length == 3
  • list[1] === undefined

For me, this looks like a sufficient reason to consider [ 1, undefined, 3 ] the valid and most intuitive output in both originally mentioned cases.
Is there ANY practical reason to distinguish between them?

@medikoo
Copy link

medikoo commented Sep 6, 2011

[ 1, undefined, 3 ] would be inconsistent, as it may show same for two different arrays:

var a = [1, undefined, 3];
console.log(1 in a); // true

var b = [1, 2, 3];
delete b[1];
console.log(1 in b); // false

Currently when we declare named property on array, we have output like following:

var a = [1, 2, 3];
a.foo = 'bar';
console.log(a); // [ 1, 2, 3, foo: 'bar' ]

That is great.

So I guess best would be to do same for "disconnected" indexes:

var a = [1, 2, 3, 4];
delete a[2];
console.log(a); // [1, 2, 4: 4]

@NuckChorris
Copy link

Either way it's icky.

On Sep 6, 2011, at 1:14 AM, [email protected] wrote:

[ 1, undefined, 3 ] would be inconsistent, as it may show same for two different arrays:

var a = [1, undefined, 3];
console.log(1 in a); // true

var b = [1, 2, 3];
delete b[1];
console.log(1 in b); // false

Currently when we declare named property on array, we have output like following:

var a = [1, 2, 3];
a.foo = 'bar';
console.log(a); // [ 1, 2, 3, foo: 'bar' ]

That is great.

So I guess best would be to do same for "disconnected" indexes:

var a = [1, 2, 3, 4];
delete a[2];
console.log(a); // [1, 2, 4: 4]

Reply to this email directly or view it on GitHub:
#1651 (comment)

@koichik
Copy link

koichik commented Sep 6, 2011

How about c6fe2b3? (This is a workaround for a workaround...)

> a = [1, 2, 3]
[ 1, 2, 3 ]
> delete a[1]
true
> a
[ 1, , 3 ]
> require('util').inspect(a, true)
'[ 1, , 3, [length]: 3 ]'

But... unfortunately:

> new Array(10)
[ , , , , , , , , ,  ]

(´・ω・`)

@NuckChorris
Copy link

I approve of that behavior.

On Sep 6, 2011, at 8:31 AM, [email protected] wrote:

How about c6fe2b3? (This is a workaround for a workaround...)

> a = [1, 2, 3]
[ 1, 2, 3 ]
> delete a[1]
true
> a
[ 1, , 3 ]
> require('util').inspect(a, true)
'[ 1, , 3, [length]: 3 ]'

But... unfortunately:

> new Array(10)
[ , , , , , , , , ,  ]

(´・ω・`)

Reply to this email directly or view it on GitHub:
#1651 (comment)

@TooTallNate
Copy link

@koichik +1

@koichik
Copy link

koichik commented Sep 7, 2011

@NuckChorris, @TooTallNate - Thanks.
@bnoordhuis - Can you review c6fe2b3?

@bnoordhuis
Copy link
Member

@koichik - LGTM but I think it's time we start breaking up that 200 line inspect function, it's getting unwieldy.

@koichik
Copy link

koichik commented Sep 8, 2011

@bnoordhuis - I agree. I will extract the anonymous function that is parameter of keys.map().

@koichik
Copy link

koichik commented Sep 8, 2011

@bnoordhuis - I have just divided util.inspect() into some subroutines.
Please review b86210f and a39c639.

@bnoordhuis
Copy link
Member

@koichik - Love it. There should be two newlines between functions but otherwise LGTM.

@koichik
Copy link

koichik commented Sep 8, 2011

There should be two newlines between functions

Done. Thanks!

@koichik koichik closed this as completed in 6139459 Sep 8, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants