Assignments week 2 Ilia Bubnov #8
Conversation
There was a problem hiding this comment.
A very strong assignment you should be proud of, some very nice features are showing through like separation of logic, and testing of edge cases.
My biggest place for improvement is how can you better handle errors so that the front-end can use the messages in a valuable/consistent way.
| res.send("hello from backend to frontend!"); | ||
| }); | ||
| app.post("/weather", (req, res) => { | ||
| getWeather(req, res); |
There was a problem hiding this comment.
This is really good seperation of logic, placing functions/controllers in a different class/method. Good Job!
| const message = { weatherText: "" }; | ||
| if (!req.body.cityName) { | ||
| message.weatherText = "You have to fill in city name"; | ||
| return res.status(400).json(message); |
There was a problem hiding this comment.
We should use a different model for errors than we do for valid responses, as the client may not check the status code, and just if their is a weatherText value. A good practise is all errors get returned in an error object like :
{
"errorCode" : "INVALID_REQUEST",
"errorMessage" : "You have to fill in a city name"
}
This allows you to build standard error wrappers/handlers in both the back-end and front-end.
| message.weatherText = | ||
| "Something went wrong on server side, try again later"; | ||
| return res.status(e.status).json(message); | ||
| } |
There was a problem hiding this comment.
If the errors status is not set (e.g. the error was thrown from something other that the API request, then this API will not respond properly, good to write at the bottom a single "unknown error". response.
| const request = supertest(app); | ||
|
|
||
| describe("POST /weather", () => { | ||
| it("Should return an error when cityName is not passed", async () => { |
| @@ -0,0 +1,3 @@ | |||
| export const keys = { | |||
There was a problem hiding this comment.
It's good practice to not push up API keys to repo's to keep them safe. two ways to do this are:
-> Ignoring this file in the .gitignore.
-> adding a .env file (and ignoring it) - This is better, as the code will still compile, and the .env file is a standard across a lot of software/programs, so loading them into the program at runtime is handleded well.
No description provided.