Parsity Weather Project Eval by bhdoggett · Pull Request #216 · projectshft/weather-project

bhdoggett · 2024-12-06T04:53:15Z

No description provided.

…e and latitude from the fetchCoordinates function. But I can't yet use this url to properly fetch the five-day data.

…te the 40 data sets into 5 sets of 8 and find the average temperature of each day. I'm working on counting the weather descriptons and icons to identify the most commonly appearing of each on each day.

… temp, the most common weather description, and the most common weather icon. Next step is to turn this data into an html tmeplate that I can append to the website.

…e I also need to updat the day of the week that is listed at the bottom.

…recast

…).then()

…te concerns of each function more clearly

…r current location, using javascript's built in geolocation funcionality to identify the user's longitude and latitude, to be used in the already constructed addNow and addFiveDay functions.

…tential extension, trying to create an auto-fill function for typing in a city/state/country

sethblanchard · 2024-12-06T15:08:34Z

main.js

@@ -0,0 +1,334 @@
+const apiKey = "6427275c4ee8b157888fdf144b2fc5ca";


usually you don't put keys in github you would read it from an ENV variable but I'm sure your instructor will go over this.

Since it's a free api subscription, they told me I could include. Haven't tackled node yet.

sethblanchard · 2024-12-06T15:10:35Z

main.js

+    maximumAge: 0,
+  };
+
+  return new Promise((resolve, reject) => {


I like that you wrap this long running process in a promise.

sethblanchard · 2024-12-06T15:11:11Z

main.js

+    const processedData = await processFiveDayData(fiveDayData);
+    addFiveDay(processedData);
+
+    addFiveDay;


seems like an accident? Should delete

oh do you mean to return it?

yeah that's an accident. I deleted.

sethblanchard · 2024-12-06T15:13:21Z

main.js

+    // identify the appropriate data range
+    let indexRange = [];
+
+    if (day === 1) {


maybe just make this a dictionary lookup

Yeah, you could store this mapping in an object like this:

const ranges = { 1: [0, 7], 2: [0, 15], ...etc } const indexRange = ranges[day];

However, lines 168-184 could be replaced with one line with simple math...

I'll hold off on giving you the solution... but I have it :D

Figured this out!

sethblanchard · 2024-12-06T15:14:51Z

main.js

+
+    <div class="row justify-content-center my-2">
+      <div class="col-md-2 container-fluid border justify-content-center" id="day-1">
+        <p class="text-center mt-2">


You could make a function that is a template of just the day of summary data and then iterate over it. Would be a lot less code and you would repeat it less

sethblanchard · 2024-12-06T15:15:08Z

main.js

+          <span class="text-center">${summaryData[0].weatherSummary}</span>
+          <br />
+          <span class="text-center"><strong>${summaryData[0].avgTemp}°</strong></span>
+          <br />


nit: no br do it with CSS 😈

I changed to using
and used bootstrap classes to handle spacing.

…ries of if statements

…ate to build the section rather than write out html for every single day in the main.js file

tommymarshall · 2024-12-06T19:35:32Z

main.js

+
+    const indexRange = [(day - 1) * 8, day * 8 - 1];
+
+    for (let i = indexRange[0]; i <= indexRange[1]; i++) {


it's really good do the hasOwnProperty check! that being said, this is a shorter way of writing it, since we essentially do the same thing twice wutgh

for (let i = indexRange[0]; i <= indexRange[1]; i++) { // once you understand this, half of your javascript knowledge is complete // the `[{ main, icon }]` will only ever take the first item in the array, so it's sort of brittle const { main: { temp }, weather: [{ main, icon }] } = data.list[i]; tempAcc += temp; weatherDescriptions[main] = (weatherDescriptions[main] || 0) + 1; icons[icon] = (icons[icon] || 0) + 1; }

…e centered and increase size of day name.

ronyrosenberg · 2024-12-12T14:21:13Z

index.html

+
+
+


beware of empty spaces

ronyrosenberg · 2024-12-12T14:21:42Z

main.js

+  });
+};
+
+async function fetchCurrentLocationData(location) {


some functions are es6, others es5, be consistent

ronyrosenberg · 2024-12-12T14:24:42Z

main.js

+async function processFiveDayData(data) {
+  const dividedDayData = [];
+
+  function processDayData(day) {


this function is too big, needs to be spread into smaller functions to help with readability and maintainability

…istinct functions. Return an object with those funcitons called as the values of the appropriate keys.

…6, arrow functions

…directly to the dividedDayData array rather than return an object to be pushed into that array at a later time.

ronyrosenberg · 2024-12-19T15:58:42Z

main.js

+    };
+
+    const getSummaryIcon = () => {
+      let icons = {};


can be const.

ronyrosenberg · 2024-12-19T16:07:17Z

main.js

+const fetchFiveDayData = async (coordinates) => {
+  const fiveDayURL = `https://api.openweathermap.org/data/2.5/forecast?lat=${coordinates.lat}&lon=${coordinates.lon}&appid=${apiKey}&units=${units}`;
+
+  const fetchFiveDay = await fetch(fiveDayURL, {


the object is static, can be a const on top of the file

ronyrosenberg · 2024-12-19T16:09:53Z

main.js

+    const processedData = await processFiveDayData(fiveDayData);
+    addFiveDay(processedData);
+  } catch (error) {
+    console.error("Error processing data:", error);


this doesnt really help the user.

ronyrosenberg · 2024-12-19T16:11:02Z

main.js

+};
+
+// Colate data from each day in the five-day forecast.
+const processFiveDayData = async (data) => {


this function is still complex, nested and logic that helpers can help with readability and debugging

bhdoggett added 17 commits November 27, 2024 19:26

Initial Commit

b28d494

Work on html template in index.html

0d86998

Continue formatting the forecast tempate in index.html

96419af

Current Weather data fetch function is working.

ea67da6

I can now create a url for the fiveDayData function with the longitud…

f5793ba

…e and latitude from the fetchCoordinates function. But I can't yet use this url to properly fetch the five-day data.

I added https:// to the fiveDayURL and now I can retrieve that data.

2bb1f62

moved api key to a seperate file that isn't tracked by git

5a6ca2e

Working on processing the 5 day forcast data. I can currently sepeare…

9565456

…te the 40 data sets into 5 sets of 8 and find the average temperature of each day. I'm working on counting the weather descriptons and icons to identify the most commonly appearing of each on each day.

I can now return an object for each day with a summary of the average…

8594840

… temp, the most common weather description, and the most common weather icon. Next step is to turn this data into an html tmeplate that I can append to the website.

I can now use the summary data to populate the html. But now I realiz…

d25f785

…e I also need to updat the day of the week that is listed at the bottom.

Update main.js to enable posting of weekday names for the five day fo…

91eaf79

…recast

Refactor to use async await in the getNow function rather than .then(…

b144c92

…).then()

Extract child functions from within getNow and getFive day and sepera…

6313d6c

…te concerns of each function more clearly

Update Code to default the page to start with the weather forecast fo…

563be18

…r current location, using javascript's built in geolocation funcionality to identify the user's longitude and latitude, to be used in the already constructed addNow and addFiveDay functions.

Minor edit to Readme

7b2e68f

Remove countries+cities.json

82eef76

Update .gitignore to ignore countries-cities. This is for a future po…

6c1e954

…tential extension, trying to create an auto-fill function for typing in a city/state/country

sethblanchard reviewed Dec 6, 2024

View reviewed changes

main.js

maximumAge: 0,

};

return new Promise((resolve, reject) => {

Copy link

sethblanchard Dec 6, 2024

Choose a reason for hiding this comment

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

I like that you wrap this long running process in a promise.

sethblanchard reviewed Dec 6, 2024

View reviewed changes

bhdoggett added 6 commits December 6, 2024 12:58

Simplify index range code in processDayData function to remove the se…

f3eb71c

…ries of if statements

Simplify five-day forecast template to iterate over a singleDay templ…

9f0420d

…ate to build the section rather than write out html for every single day in the main.js file

Remove console logs and comments

14588df

Remove style.css. Unnecessary because bootrap is used for styling.

06958ea

Delete unneccessary 'addFiveDay'in previous line 48

ea295f8

No longer need .gitignore file

c72e3de

tommymarshall reviewed Dec 6, 2024

View reviewed changes

Fix five-day forecast section's day template to properly display imag…

f38cc6a

…e centered and increase size of day name.

ronyrosenberg reviewed Dec 12, 2024

View reviewed changes

bhdoggett added 5 commits December 12, 2024 16:23

Update processDayData function to separate multiple operations into d…

253f3cd

…istinct functions. Return an object with those funcitons called as the values of the appropriate keys.

Update Readme and update main.js to standardize function syntax to ES…

c9720d2

…6, arrow functions

Fix bug in processisng avg temp

b0e4d50

Remove extra space in index.html and refactor processDayData to push …

c5b97f4

…directly to the dividedDayData array rather than return an object to be pushed into that array at a later time.

Update comments

e38200a

ronyrosenberg reviewed Dec 19, 2024

View reviewed changes

		@@ -0,0 +1,334 @@
		const apiKey = "6427275c4ee8b157888fdf144b2fc5ca";


		const indexRange = [(day - 1) * 8, day * 8 - 1];

		for (let i = indexRange[0]; i <= indexRange[1]; i++) {

Conversation

bhdoggett commented Dec 6, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greypants Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tommymarshall Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

greypants Dec 6, 2024 •

edited

Loading

tommymarshall Dec 6, 2024 •

edited

Loading