Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link
  • Never indent as you have done. This make code very unreadable due to the long lines.

Map.values() creates an iteratable object. It does not require an array to be created and is \$O(1)\$ storage as it iterates over the already stored values

You have forced the map to be stored as an array. Array.from(map.values()) That means that it must be iterated and stored \$O(n)\$ complexity and storage best case. If you just kept the map and iterated to find the result you could have a best case of \$O(1)\$ to find the result.

Always iterate iteratable objects to reduce memory uses and if there is an early exit to reduce CPU use and complexity.

##Rewrite

Rewrite

Is around 2* as fast depending on input

function firstUniqChar(str) {
 const counts = new Map();
 var idx = 0;
 for (const c of str) { // iterate to avoid storing array of characters
 if (counts.has(c)) { counts.get(c).count ++ }
 else { counts.set(c, {idx, count: 1}) }
 idx++;
 }
 for (const c of counts.values()) {
 if (c.count === 1) { return c.idx }
 }
 return - 1;
}
  • Never indent as you have done. This make code very unreadable due to the long lines.

Map.values() creates an iteratable object. It does not require an array to be created and is \$O(1)\$ storage as it iterates over the already stored values

You have forced the map to be stored as an array. Array.from(map.values()) That means that it must be iterated and stored \$O(n)\$ complexity and storage best case. If you just kept the map and iterated to find the result you could have a best case of \$O(1)\$ to find the result.

Always iterate iteratable objects to reduce memory uses and if there is an early exit to reduce CPU use and complexity.

##Rewrite

Is around 2* as fast depending on input

function firstUniqChar(str) {
 const counts = new Map();
 var idx = 0;
 for (const c of str) { // iterate to avoid storing array of characters
 if (counts.has(c)) { counts.get(c).count ++ }
 else { counts.set(c, {idx, count: 1}) }
 idx++;
 }
 for (const c of counts.values()) {
 if (c.count === 1) { return c.idx }
 }
 return - 1;
}
  • Never indent as you have done. This make code very unreadable due to the long lines.

Map.values() creates an iteratable object. It does not require an array to be created and is \$O(1)\$ storage as it iterates over the already stored values

You have forced the map to be stored as an array. Array.from(map.values()) That means that it must be iterated and stored \$O(n)\$ complexity and storage best case. If you just kept the map and iterated to find the result you could have a best case of \$O(1)\$ to find the result.

Always iterate iteratable objects to reduce memory uses and if there is an early exit to reduce CPU use and complexity.

Rewrite

Is around 2* as fast depending on input

function firstUniqChar(str) {
 const counts = new Map();
 var idx = 0;
 for (const c of str) { // iterate to avoid storing array of characters
 if (counts.has(c)) { counts.get(c).count ++ }
 else { counts.set(c, {idx, count: 1}) }
 idx++;
 }
 for (const c of counts.values()) {
 if (c.count === 1) { return c.idx }
 }
 return - 1;
}
Source Link
Blindman67
  • 22.8k
  • 2
  • 16
  • 40
  • Never indent as you have done. This make code very unreadable due to the long lines.

Map.values() creates an iteratable object. It does not require an array to be created and is \$O(1)\$ storage as it iterates over the already stored values

You have forced the map to be stored as an array. Array.from(map.values()) That means that it must be iterated and stored \$O(n)\$ complexity and storage best case. If you just kept the map and iterated to find the result you could have a best case of \$O(1)\$ to find the result.

Always iterate iteratable objects to reduce memory uses and if there is an early exit to reduce CPU use and complexity.

##Rewrite

Is around 2* as fast depending on input

function firstUniqChar(str) {
 const counts = new Map();
 var idx = 0;
 for (const c of str) { // iterate to avoid storing array of characters
 if (counts.has(c)) { counts.get(c).count ++ }
 else { counts.set(c, {idx, count: 1}) }
 idx++;
 }
 for (const c of counts.values()) {
 if (c.count === 1) { return c.idx }
 }
 return - 1;
}
lang-js

AltStyle によって変換されたページ (->オリジナル) /