Skip to content

Commit 1bcda5e

Browse files
jesus-seijas-spjasnell
authored andcommitted
util: refactor format method.Performance improved.
Method format refactored to make it more maintenable, replacing the switch by a function factory, that returns the appropiated function given the character (d, i , f, j, s). Also, performance when formatting an string that contains several consecutive % symbols is improved. The test: `const numSamples = 10000000; const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%'; var s; console.time('Percents'); for (let i = 0; i < numSamples; i++) { s = util.format(strPercents, 'test', 12); } console.timeEnd('Percents');` Original time: 28399.708ms After refactor: 23763.788ms Improved: 16% PR-URL: #12407 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Brian White <mscdex@mscdex.net>
1 parent dcadeb4 commit 1bcda5e

File tree

2 files changed

+22
-26
lines changed

2 files changed

+22
-26
lines changed

lib/util.js

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -68,71 +68,59 @@ exports.format = function(f) {
6868
return objects.join(' ');
6969
}
7070

71-
var argLen = arguments.length;
72-
73-
if (argLen === 1) return f;
71+
if (arguments.length === 1) return f;
7472

7573
var str = '';
7674
var a = 1;
7775
var lastPos = 0;
7876
for (var i = 0; i < f.length;) {
7977
if (f.charCodeAt(i) === 37/*'%'*/ && i + 1 < f.length) {
78+
if (f.charCodeAt(i + 1) !== 37/*'%'*/ && a >= arguments.length) {
79+
++i;
80+
continue;
81+
}
8082
switch (f.charCodeAt(i + 1)) {
8183
case 100: // 'd'
82-
if (a >= argLen)
83-
break;
8484
if (lastPos < i)
8585
str += f.slice(lastPos, i);
8686
str += Number(arguments[a++]);
87-
lastPos = i = i + 2;
88-
continue;
87+
break;
8988
case 105: // 'i'
90-
if (a >= argLen)
91-
break;
9289
if (lastPos < i)
9390
str += f.slice(lastPos, i);
9491
str += parseInt(arguments[a++]);
95-
lastPos = i = i + 2;
96-
continue;
92+
break;
9793
case 102: // 'f'
98-
if (a >= argLen)
99-
break;
10094
if (lastPos < i)
10195
str += f.slice(lastPos, i);
10296
str += parseFloat(arguments[a++]);
103-
lastPos = i = i + 2;
104-
continue;
97+
break;
10598
case 106: // 'j'
106-
if (a >= argLen)
107-
break;
10899
if (lastPos < i)
109100
str += f.slice(lastPos, i);
110101
str += tryStringify(arguments[a++]);
111-
lastPos = i = i + 2;
112-
continue;
102+
break;
113103
case 115: // 's'
114-
if (a >= argLen)
115-
break;
116104
if (lastPos < i)
117105
str += f.slice(lastPos, i);
118106
str += String(arguments[a++]);
119-
lastPos = i = i + 2;
120-
continue;
107+
break;
121108
case 37: // '%'
122109
if (lastPos < i)
123110
str += f.slice(lastPos, i);
124111
str += '%';
125-
lastPos = i = i + 2;
126-
continue;
112+
break;
127113
}
114+
lastPos = i = i + 2;
115+
continue;
128116
}
129117
++i;
130118
}
131119
if (lastPos === 0)
132120
str = f;
133121
else if (lastPos < f.length)
134122
str += f.slice(lastPos);
135-
while (a < argLen) {
123+
while (a < arguments.length) {
136124
const x = arguments[a++];
137125
if (x === null || (typeof x !== 'object' && typeof x !== 'symbol')) {
138126
str += ' ' + x;

test/parallel/test-util-format.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ assert.strictEqual(util.format('%%s%s', 'foo'), '%sfoo');
106106
assert.strictEqual(util.format('%s:%s'), '%s:%s');
107107
assert.strictEqual(util.format('%s:%s', undefined), 'undefined:%s');
108108
assert.strictEqual(util.format('%s:%s', 'foo'), 'foo:%s');
109+
assert.strictEqual(util.format('%s:%i', 'foo'), 'foo:%i');
110+
assert.strictEqual(util.format('%s:%f', 'foo'), 'foo:%f');
109111
assert.strictEqual(util.format('%s:%s', 'foo', 'bar'), 'foo:bar');
110112
assert.strictEqual(util.format('%s:%s', 'foo', 'bar', 'baz'), 'foo:bar baz');
111113
assert.strictEqual(util.format('%%%s%%', 'hi'), '%hi%');
@@ -114,6 +116,12 @@ assert.strictEqual(util.format('%sbc%%def', 'a'), 'abc%def');
114116
assert.strictEqual(util.format('%d:%d', 12, 30), '12:30');
115117
assert.strictEqual(util.format('%d:%d', 12), '12:%d');
116118
assert.strictEqual(util.format('%d:%d'), '%d:%d');
119+
assert.strictEqual(util.format('%i:%i', 12, 30), '12:30');
120+
assert.strictEqual(util.format('%i:%i', 12), '12:%i');
121+
assert.strictEqual(util.format('%i:%i'), '%i:%i');
122+
assert.strictEqual(util.format('%f:%f', 12, 30), '12:30');
123+
assert.strictEqual(util.format('%f:%f', 12), '12:%f');
124+
assert.strictEqual(util.format('%f:%f'), '%f:%f');
117125
assert.strictEqual(util.format('o: %j, a: %j', {}, []), 'o: {}, a: []');
118126
assert.strictEqual(util.format('o: %j, a: %j', {}), 'o: {}, a: %j');
119127
assert.strictEqual(util.format('o: %j, a: %j'), 'o: %j, a: %j');

0 commit comments

Comments
 (0)