yasershomaf/movies

Homework for HackYourFutre week 5

movies's Issues

Don't use alert

They can be disabled and they are not very user-friendly. Just display a warning like you do with the 'no results found' message.

Awesome!!!

You made a very awesome use of the extra parameters pageIndex! Dynamically looking for the next set of pages is great!

Missing part of the assignment

Point 5, the hardest of all is missing.

arrayToUnique.js

• Syntax: Put spaces between operators i < 0 instead of i<0 and i === ii instead of i===ii.
• Unnecessary condition. The first if is not needed, if length is 0 it would loop 0 times. You could have put a check in like (Array.isArray(arr) or arr == null)
• Most important: you are modifying the array you are looping over. It would be better to make a new array where you push the results into or make a shallow copy (arr.slice())

Using innerHtml

Sometimes text nodes are created, sometimes innerHtml is set. Be consistent, and just append text nodes.

Image handling

Great way how you handled missing images!

General style

• Don't put all code in one giant function, this is hard to read. Try to seperate these things into multiple functions: one that handles the request, one that creating of the main content, one that handles creating each sub element being created etc.

  Edit: You did that actually but since they are all contained in a big function (which is not necessarily bad) but it's quite hard to read and too much is happening so the overview is hard. Putting comments between your code, commenting your functions would help a lot. Also seperate statements which belong together logically together, and then place a newline.

• file.js is a very non-descriptive file name. It tells us nothing about what it wants to do, and basically everything on your comuter is a file.

• You are referencing variables from your script in your HTML. This is not needed at all. You could better make seperate functions (one which sets the last page) and then calls the request function.

• The arrows on your HTML page are not very descriptives of what they do. Normally it's two types, one that goes to the end and one that goes one page further. I had to click them to really figure them out ^^

Going to next page takes current (changed) value

When I go to the next word but accidentally changed the content of the search bar while not wanting to search for it it takes the new value.

You could have stored the current 'query' in the document, just as you do with currentpages etc.