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

querystring: do not display separator for empty array stringify #7975

Closed
wants to merge 2 commits into from
Closed

Conversation

cjihrig
Copy link

@cjihrig cjihrig commented Jul 19, 2014

Currently, stringification of an empty array outputs a single separator character. This commit causes an empty array to output the empty string. Also removed an unnecessary if statement. Related to #7971

@danielb2
Copy link

👍

var keys = Object.keys(obj);
var fields = [];

for (var i = 0, il = keys.length; i < il; ++i) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not cache keys.length, there is no point in it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i++

cjihrig added 2 commits July 23, 2014 15:58
Currently, stringification of an empty array outputs a single
separator character. This commit causes an empty array to output
the empty string.
@cjihrig
Copy link
Author

cjihrig commented Jul 23, 2014

@indutny I have made the requested changes. I added two additional tests to protect against regressions where the object is null or undefined.

As for not caching the array length in the loop - I've heard of that as good practice. Is your reason for leaving it out because V8 is smart enough to cache the value on its own?

@indutny
Copy link
Member

indutny commented Jul 23, 2014

@cjihrig yes. This practice is no longer relevant AFAIK, btw.

@indutny
Copy link
Member

indutny commented Jul 23, 2014

Landed in 61ddad1, thank you!

@indutny indutny closed this Jul 23, 2014
@danielb2
Copy link

👍

@cjihrig cjihrig deleted the 7971 branch July 23, 2014 21:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants