Skip to content

FEAT-3: change direction labels to use 16-point compass rose directions #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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 25, 2024
Merged

FEAT-3: change direction labels to use 16-point compass rose directions #7

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bbc2fc
fix: change direction labels to use 16-point compass rose directions
NickAnderegg Apr 22, 2024
a0dc764
feat: adjust center of compass point range
NickAnderegg Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions src/weatherAPI.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,40 +33,24 @@ const weatherCodes = {
99: 'Thunderstorm',
}

// A 32-point compass rose
// A 16-point compass rose, where each point is 22.5 degrees from the previous.
const compassPoints = [
['North', 'N'],
['North by East', 'NbE'],
['North-Northeast', 'NNE'],
['Northeast by North', 'NEbN'],
['Northeast', 'NE'],
['Northeast by East', 'NEbE'],
['East-Northeast', 'ENE'],
['East by North', 'EbN'],
['East', 'E'],
['East by South', 'EbS'],
['East-Southeast', 'ESE'],
['Southeast by East', 'SEbE'],
['Southeast', 'SE'],
['Southeast by South', 'SEbS'],
['South-Southeast', 'SSE'],
['South by East', 'SbE'],
['South', 'S'],
['South by West', 'SbW'],
['South-Southwest', 'SSW'],
['Southwest by South', 'SWbS'],
['Southwest', 'SW'],
['Southwest by West', 'SWbW'],
['West-Southwest', 'WSW'],
['West by South', 'WbS'],
['West', 'W'],
['West by North', 'WbN'],
['West-Northwest', 'WNW'],
['Northwest by West', 'NWbW'],
['Northwest', 'NW'],
['Northwest by North', 'NWbN'],
['North-Northwest', 'NNW'],
['North by West', 'NbW']
]

// Example Open-Meteo API query showing how the parameters are used:
Expand Down Expand Up @@ -170,7 +154,7 @@ function generateCurrentConditions(weatherData) {
}
})

const windCompass = compassPoints[Math.round(rawConditions.wind_direction_10m / 11.25)]
const windCompass = compassPoints[Math.round((rawConditions.wind_direction_10m - 11.25) / 22.5)]
currentConditions['wind_compass'] = windCompass[0]
currentConditions['wind_compass_short'] = windCompass[1]

Copy link

Choose a reason for hiding this comment

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

The patch seems to reduce the points in the compass representation from a 32-point system to a 16-point system. However, this modification introduces potential issues:

  1. Array Index Out of Bounds: The wind direction calculation is changed from Math.round(rawConditions.wind_direction_10m / 11.25) to Math.round(rawConditions.wind_direction_10m / 22.5). This changes the scale so it will potentially result in index values higher than the size of the new compassPoints array. With the old 32-point scale, you would get index values ranging from 0 up to 31. With the new 16-point scale, the maximum would be around 16 which is out of bounds for compassPoints, considering indices range from 0 to 15.

To fix this issue, a careful redesign of the wind direction calculation might be needed, keeping in mind that the wind direction varies from 0-360 degrees. For example, code could use something like Math.floor(rawConditions.wind_direction_10m / 22.5) % 16.

  1. Breaking Changes: Besides, it's possible these changes can cause breaking changes elsewhere where other functions or services are expecting the original 32-point compass data and not the new simplified version. This may impact any feature depending on specific compass point callbacks such as 'North by West', etc., which have been removed.

To resolve this, it would be advised to conduct a thorough utilization analysis of the compassPoints array and its dependent features across the entire codebase.

Copy link

Choose a reason for hiding this comment

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

The patch reduces the compass points from a 32-point system to a 16-point system—reducing the granularity of wind direction data. Here are the potential issues:

  1. Index Out Of Range: The change in lines from const windCompass = compassPoints[Math.round(rawConditions.wind_direction_10m / 11.25)] to const windCompass = compassPoints[Math.round((rawConditions.wind_direction_10m - 11.25) / 22.5)] may result in an "index out of range" exception or error. This will happen when rawConditions.wind_direction_10m is less than 11.25, because the index returned can be -1, and there's no compassPoints[-1].

  2. Underflow Error: When rawConditions.wind_direction_10m (direction) subtracts 11.25, if the direction is less than 11.25, this could potentially have unintended side effects or even lead to an underflow error.

  3. Loss of Precision: Reducing the compass points from 32 to 16 results in a loss of precision for determining wind direction.

Suggestions:

  • Adding boundary-checking code before attempting to access the array can help avoid exception errors.
  • The equation should handle cases where wind_direction_10m is less than 11.25 properly.
  • Stick to the 32-point compass system if that level of precision in wind direction information is needed.

Expand Down