hellagood9 / capstone-project-2-expresso Goto Github PK

Great work--your solution passes all the specs, behaves as expected in the browser, and your code is clean and readable!

General comments:

• Good folder structure--individual routers are separated into their own files in the api/ folder.
• Good use of middleware for validation and router.param for clean code
• There are a few tricks you can use to clean up your code even more using some special sauce from ES6 and Node. Check out the other Issues for suggestions.

I've left a few comments in the Issues tab. In general, your code is very clean and readable, but there are some minor formatting issues. Some might be a little opinionated--if this is the case, feel free to ignore!

This comment may be less actionable, but I wanted to call your attention to it as something to keep thinking about as you code--developers have various opinions on code comments and when to use them. I'd recommend this article from the past year for some good context:

https://medium.freecodecamp.org/code-comments-the-good-the-bad-and-the-ugly-be9cc65fbf83

I'm mentioning this because you've done a great job creating a working and clean solution, and I think it's a great time to continue thinking about more advanced concepts, like how to make your code readable and self-documenting. I think you've done a good job here, so I wonder if comments like this are even necessary, given that the method itself describes its behavior. There's not necessarily a right or wrong answer here, but it's something to keep in mind as you code!

https://github.com/hellagood9/capstone-project-2-expresso/blob/master/migration.js#L4-L36

This is a very minor suggestion--Since the SQL multiline queries have similar indentation style to JS code blocks, I like to indent another level on the query and match the closing ) on the db methods with the indentation level of the call.

Ex.:

db.run(`CREATE TABLE IF NOT EXISTS Employee (
    id INTEGER PRIMARY KEY,
    name TEXT NOT NULL,
    position TEXT NOT NULL,
    wage INTEGER NOT NULL,
    is_current_employee INTEGER DEFAULT 1
  )`
);

Here's another small thing from ES6 that you use some places but not others.

In a situation like this, you can use object property shorthand to create the json response object like so:

res.status(200).json({ timesheets });

which is equivalent to the code in your repo.

It's a small thing, but it can help keep your code compact, and ES6 syntax is now common enough that this shouldn't present much of a readability issue to other developers.

I would suggest using logging middleware for your API. You don't have to use morgan--there are many others and many customizations.

First, when developing, it's helpful to receive server feedback about received requests. It can be an invaluable debugging tool for testing your route handlers, with a front-end, or with a tool like Postman.

In production development, server logs are often saved to files or databases, which again make for easier debugging and help alert engineers when something goes wrong.

This is a pretty minor suggestion, but also utilizes a nice trick about Node's require.

In the api/ folder, you have a file named api.js, meaning that you require it as '/api/api' in server.js (Link).

In this situation, where you have an entry point file for a folder like api/api.js, you can change the filename to index.js, and by default, Node will find this file if you require the whole folder (e.g. require('/api') in server.js). An index.js file is also a signal to other developers that that particular file should be the entry point to the folder, and it will likely be responsible for requiring and using code from the other files.

For reference, here's probably more than you want to know about the inner workings of requrie