Skip to content
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

Feature: Added celsius support for weather #19

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Feature: Added celsius support for weather #19

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4abcbd2
Feature: Added celsius support for weather
Aug 1, 2016
477e33b
Fix: Added documentation
Aug 1, 2016
a50d279
Fix: Fixed documentation spacing
Aug 1, 2016
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
2 changes: 1 addition & 1 deletion public/javascript/bot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ function messageHandler(event) {
handlerFunctions['getPokemon'](userAPI, event.threadID, event.senderID, message);
} else if ((/^@stock .+$/).test(message)) {
handlerFunctions['getStock'](userAPI, event.threadID, message);
} else if ((/^@weather ([0-9]{5}|([a-zA-Z ]+(, )?[a-zA-Z ]+))$/).test(message)) {
} else if ((/^@weather ([0-9]{5} [CF]|([a-zA-Z ]+(, )?[a-zA-Z ]+))$/).test(message)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you make the degree type into flags --unit <C/F> and -u <C/F>? And modify the regex accordingly to sanitize the input.

Copy link
Author

Choose a reason for hiding this comment

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

Not quite sure how to do that? Any tutorials?

Copy link
Author

Choose a reason for hiding this comment

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

or you want a format like @weather 78701 --unit C

Copy link
Owner

@renxinhe renxinhe Aug 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah like that. @weather 78701 --unit C, or @weather 78701 -u F, or lower cases.

If we use flags, we are guaranteed that the characters after the - are not part of the address, so we can simply substring the flag out, and use the @weather 78701 part of the body the old way.

handlerFunctions['getWeather'](userAPI, event.threadID, message);
} else if ((/^@meme$/).test(message)) {
handlerFunctions['rickroll'](userAPI, event.threadID);
Expand Down
23 changes: 18 additions & 5 deletions public/javascript/getWeather.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,22 @@
var weather = require("weather-js");

module.exports = function getWeather(api, threadID, body) {
const locale = body.substring('@weather '.length);
console.log('Fetching weather for ' + locale + '...');
weather.find({ search: locale, degreeType: 'F' }, function(err, result) {
var locale = '';
var currentDType = 'F';
const infoArray = body.split(' ');
for (var i = 1; i < infoArray.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop seems like a lot of work for just basically checking if the last thing is a C or an F.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I was thinking about extracting the last part first and process it afterwards.

if (i == 1) {
locale += infoArray[i];
} else {
if (i != infoArray.length - 1 || (i == infoArray.length - 1 && infoArray[infoArray.length - 1] != 'F' && infoArray[infoArray.length - 1] != 'C')) {
locale += ' ' + infoArray[i];
} else {
currentDType = infoArray[infoArray.length - 1];
}
}
}
console.log('Fetching weather for ' + locale + ' with degreeType ' + currentDType + '...');
weather.find({ search: locale, degreeType: currentDType }, function(err, result) {
if (err) {
api.sendMessage(err, threadID);
console.error(err);
Expand Down Expand Up @@ -65,9 +78,9 @@ module.exports = function getWeather(api, threadID, body) {
} else {
emoji = '';
}

var displayType = currentDType === 'F' ? '\xB0F\n' : '\xB0C\n';
Copy link
Contributor

Choose a reason for hiding this comment

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

var displayType = '\xB0' + currentDType + '\n';
is cleaner lol.

Copy link
Author

Choose a reason for hiding this comment

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

👍

Copy link
Author

Choose a reason for hiding this comment

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

I think it can be a const as well right? Considering we aren't changing the display degree type afterwards.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably

let message =
data.current.temperature + '\xB0F\n' + data.location.name + '\n' + data.current.skytext + emoji + '\n' +
data.current.temperature + displayType + data.location.name + '\n' + data.current.skytext + emoji + '\n' +
'Feels like ' + data.current.feelslike + '\xB0. Humidity ' + data.current.humidity + '%.\n';

// Concatenate forecast to message
Expand Down