Skip to content

ToCode Solution: תרגול מערכים ואובייקטים#2

Open
shlomitha wants to merge 1 commit intoynonp:masterfrom
shlomitha:sol-html5-web-development.arrays-objects-lab
Open

ToCode Solution: תרגול מערכים ואובייקטים#2
shlomitha wants to merge 1 commit intoynonp:masterfrom
shlomitha:sol-html5-web-development.arrays-objects-lab

Conversation

@shlomitha
Copy link
Copy Markdown

No description provided.

- * and prints their amount
- */
function sum(){
var arr= [].slice.call(arguments);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Since you're using reduce and arrow functions (which is awesome, but is only supported in newer browsers), you might as well use the nicer:

var arr = Array.from(arguments)

Or even better:

function sum(...args) { 
  // now args is a real array
  return args.reduce((a,b) => a+b, 0); 
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Also note the second argument to reduce is the initial value, which would save you the condition in the following line

if (arr.length===0 )
console.log(0);
else{
console.log(arr.reduce((a,b)=>a+b));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Returning the value is better than printing it

function uniq(){
var duplicatesArray= [].slice.call(arguments);
var uniqueArray = duplicatesArray.filter(function(elem, pos) {
return duplicatesArray.indexOf(elem) == pos;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not a good idea.

The filter runs over all elements in the original array for each item, resulting in O(n^2) complexity.

JavaScript objects can be used as hash maps, which gives you much better performance and cleaner code. Remember that:

found = {};
found['foo'] = true

// now foo is in the map found
typeof found['foo'] !== 'undefined'

// and bar is not
typeof found['bar'] === 'undefined'

Please Fix:

  1. Don't duplicate the array upfront
  2. Use an object to remember which items were found
  3. Create the new array of unique values as you iterate the original one

This should result in O(n) complexity and nicer code.

(*) Bonus: Read on maps here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map

var uniqueArray = duplicatesArray.filter(function(elem, pos) {
return duplicatesArray.indexOf(elem) == pos;
});
console.log(uniqueArray);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Better to return a value than to print it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants