cmda-bt / be-course-17-18

🎓 Backend · 2017-2018 · Curriculum and Syllabus 💾
Other
47 stars 19 forks source link

Listing assignment #531

Closed daoneandonly closed 6 years ago

daoneandonly commented 6 years ago

Met wat vertraging hier is mijn listing assignment. Ik heb er wat extra dingen ingestopt zoals het checken of er een .html file bestaat, wanneer er geen extension name in de URL wordt opgegeven.

Link

wooorm commented 6 years ago

Awesome work @daoneandonly!

Here’s some general unordered feedback points!

Smart to read the directory, and check if it contains an index.html in the resulting list of files! I just tried reading the index.html in that case, sending it back it if it did, and only then checked the contents of the directory!

If you’re going to send errors, e.g., res.end('error dir not found'), you should set the status code to something (in this case, node said it was a directory, but later couldn’t find it, so maybe that’s an error in the 500 range?)

Always handle errors (such as here), it could be that thousands of people tried reading the file at the same time causing the computer to stop working properly, so you should send some 500 error back here.

Cool that you added the Directory is empty message! 👍

Make sure to use indentation properly (such as here), sometimes it’s hard to read where stuff starts and ends!

I don’t think you need this part anymore, because if / is request, it’s a directory, and then there’s already code in there to handle those!

readme looks nice!

Tip: don’t include node_modules in git, instead, add them to .gitignore so they‘re always ignored!