Skip to content

Assignments week 2 Oleksandr Starshynov #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aleksandrstarshynov
Copy link

Updated weather app, week 2, Node module

@aleksandrstarshynov aleksandrstarshynov changed the title Assignments week 1 Oleksandr Starshynov Assignments week 2 Oleksandr Starshynov May 2, 2025
Copy link

@EdwardSmart98 EdwardSmart98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Strong attempt for the solution, a very strong test suite aswell to go along covering a lot of edge cases.
The biggest area for improvement is in the error handling, you should try to give consistent and dexript errors to the front-end so they can understand why the error occurred.

}
catch {
console.error("Error fetching data");
res.status(500).json({ error: "Internal server error" });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is lacking in information the front-end can accurately use, is the error because the API call failed? or is the name of the city not a real city?
You can check the response from the weather API, and if it's 404, return an error with that info, so the front-end can display this to the user and the user will type a different name

@@ -0,0 +1 @@
export const keys = {API_KEY: "c34f56e7a569f2c2cf3e24aceac5ddc8"};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

…-Node

move to correctly named directory without space in name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants