chore: prefer string templates over string concatenation

[pkuczynski]

String templates are more readable, compact, and a bit faster than string concatenation. Edding eslint rule to force them.

See test results here:
https://jsben.ch/dWhr2

Or benchmark yourself:

const Benchmark = require('benchmark')
const suite = new Benchmark.Suite()
const text = 'some text'
const number = 12

suite.add('concatenation', () => { const output = 'prefix' + text + 'middle' + number + 'postfix' })
suite.add('template', () => {const output = 'prefix' + text + 'middle' + number + 'postfix' })
suite.on('cycle', (event) => { console.log(String(event.target)) })
suite.on('complete', function() { console.log('Fastest is ' + this.filter('fastest').map('name')) })
suite.run({ async: true })

My results in node:

concatenation x 690,777,686 ops/sec ±1.55% (83 runs sampled)
template x 694,885,002 ops/sec ±1.58% (84 runs sampled)
Fastest is template,concatenation

[codecov]

Codecov Report

Merging #732 (edcea9e) into main (cb746cb) will decrease coverage by 0.01%.
The diff coverage is 97.56%.

❗ Current head edcea9e differs from pull request most recent head e4f2b75. Consider uploading reports for the commit e4f2b75 to get more accurate results

@@            Coverage Diff             @@
##             main     #732      +/-   ##
==========================================
- Coverage   99.35%   99.34%   -0.02%     
==========================================
  Files        1922     1921       -1     
  Lines      183098   179122    -3976     
  Branches      902      893       -9     
==========================================
- Hits       181924   177945    -3979     
- Misses       1118     1121       +3     
  Partials       56       56              

Impacted Files	Coverage Δ
src/name.ts	98.57% <75.00%> (ø)
src/finance.ts	99.29% <93.33%> (-0.02%)	⬇️
src/address.ts	99.81% <100.00%> (ø)
src/commerce.ts	100.00% <100.00%> (ø)
src/datatype.ts	100.00% <100.00%> (ø)
src/fake.ts	100.00% <100.00%> (ø)
src/git.ts	98.30% <100.00%> (ø)
src/helpers.ts	99.27% <100.00%> (ø)
src/image.ts	98.83% <100.00%> (ø)
src/image_providers/lorempicsum.ts	94.44% <100.00%> (ø)
... and 17 more

[pkuczynski]

Needs #733 no to fail...

[Shinigami92]

Please also update vendor files, we could rename the folder later if you like to

[pkuczynski]

Please also update vendor files, we could rename the folder later if you like to

Check #733 which excludes vendor from eslint... This should be sufficient, right?

[pkuczynski]

Please also update vendor files, we could rename the folder later if you like to

Done...

[ST-DDT]

The performance gain is minimal and it has some implications on code readability.
Note sure, about it.

[pkuczynski]

The performance gain is minimal and it has some implications on code readability.
Note sure, about it.

The performance gain is minimal, but the readability increases big time. At least to my taste.

[ST-DDT]

The performance gain is minimal and it has some implications on code readability.
Note sure, about it.

The performance gain is minimal, but the readability increases big time. At least to my taste.

Yes:

-  writeFileSync(resolve(pathOutputDir, methodName + '.md'), content);
+  writeFileSync(resolve(pathOutputDir, `${methodName}.md`), content);

Maybe:

-    return (
-      this.faker.commerce.productAdjective() +
-      ' ' +
-      this.faker.commerce.productMaterial() +
-      ' ' +
-      this.faker.commerce.product()
-    );
+    return `${this.faker.commerce.productAdjective()} ${this.faker.commerce.productMaterial()} ${this.faker.commerce.product()}`;

(We have line breaks for a reason)

Not so much:

-    examples: mdToHtml('```ts\n' + examples + '```'),
+    examples: mdToHtml(`\`\`\`ts\n${examples}\`\`\``),

(Which characters are escaped?)

Not at all:

-    return (
-      this.Helpers.replaceSymbols('???') +
-      this.faker.random.arrayElement(vowels) +
-      this.faker.random.arrayElement(this.ibanLib.iso3166) +
-      this.Helpers.replaceSymbols('?') +
-      '1' +
-      (prob < 10
-        ? this.Helpers.replaceSymbols(
-            '?' + this.faker.random.arrayElement(vowels) + '?'
-          )
-        : prob < 40
-        ? this.Helpers.replaceSymbols('###')
-        : '')
-    );
+    return `${
+      this.Helpers.replaceSymbols('???') +
+      this.faker.random.arrayElement(vowels) +
+      this.faker.random.arrayElement(this.ibanLib.iso3166) +
+      this.Helpers.replaceSymbols('?')
+    }1${
+      prob < 10
+        ? this.Helpers.replaceSymbols(
+            `?${this.faker.random.arrayElement(vowels)}?`
+          )
+        : prob < 40
+        ? this.Helpers.replaceSymbols('###')
+        : ''
+    }`;

(Because now you use nested conditional string templates in combination with string concatenation.)

See above

[ST-DDT]

One last quibble, otherwise it looks good to me.

changes applied

All changes applied

[pkuczynski]

@ST-DDT @Shinigami92 all changes applied... Can you re-review, please?