viewProducts function improvements

[mattlub]

Beauty/public/main.js

Lines 41 to 68 in 9b9fd91

	function viewProducts(array) {
	array.map(element => {
	var product = document.createElement('div');
	product.setAttribute('class', 'container-div');
	mydiv.appendChild(product);
	var brand = document.createElement('h1');
	brand.setAttribute('class', 'div-hd1');
	brand.innerText = element.brand;
	var name = document.createElement('h2');
	name.setAttribute('class', 'div-hd2');
	name.innerText = element.name;
	var image = document.createElement('IMG');
	image.setAttribute('class', 'product-img');
	image.src = element.image;
	image.alt = "product image"
	var price = document.createElement('h5');
	price.setAttribute('class', 'div-hd5');
	price.innerText = element.price + element.currency;
	product.appendChild(brand);
	product.appendChild(name);
	product.appendChild(image);
	product.appendChild(price);
	});
	//clear search box
	inp.value = '';
	}

Nice work splitting up the code into functions, I have a few questions for you regarding this function:

• what would be a more suitable method than map? Why?
• does the "clear search box" functionality belong inside this function?
• how many times does your function append elements to the DOM? Could you do fewer?
• Could the function be split further? e.g. with a createProductDOMNode function, like in the todo list project.

[RazanTayyem]

what would be a more suitable method than map? Why?
No
does the "clear search box" functionality belong inside this function?
No , it should be some where else

how many times does your function append elements to the DOM? Could you do fewer?
we see no , but if you have another way tell us please

Could the function be split further? e.g. with a createProductDOMNode function, like in the todo list project.
we can do this , "was not it so clear in the way it is?"

[mattlub]

what would be a more suitable method than map? Why?
No

In this situation forEach is better semantically, because you are not using the result. Usually we use map like:

const doubled = [1, 2, 3].map(x => 2*x)

i.e. when we are interested in the result (doubled in this case)

[mattlub]

how many times does your function append elements to the DOM? Could you do fewer?
we see no , but if you have another way tell us please

for each element of your array, you append to the DOM 5 times (line 47, 61, 62, 63, 64)
so quite a lot of times in total

In general it is better to reduce this number as it is quite an expensive process.

A better way is:

const parent = document.getElementById('parent')
const container = document.createElement('div')

results.forEach(result => {
  const resultNode = createResultDOMNode(result)
  container.appendChild(resultNode)
})
parent.appendChild(container)

This only appends directly to the DOM once, in the last line.

[mattlub]

we can do this , "was not it so clear in the way it is?"

For me, lines 43-64 are not very clear, and using a createProductDOMNode function would reduce these 20 lines to less than 5 :)