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 <[email protected]> Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Brian White <[email protected]>
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
returnobjects.join(' ');
6969
}
7070

71-
varargLen=arguments.length;
72-
73-
if(argLen===1)returnf;
71+
if(arguments.length===1)returnf;
7472

7573
varstr='';
7674
vara=1;
7775
varlastPos=0;
7876
for(vari=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
case100: // '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
case105: // '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
case102: // '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
case106: // '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
case115: // '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
case37: // '%'
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
elseif(lastPos<f.length)
134122
str+=f.slice(lastPos);
135-
while(a<argLen){
123+
while(a<arguments.length){
136124
constx=arguments[a++];
137125
if(x===null||(typeofx!=='object'&&typeofx!=='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)