Use weather API to get information in JavaScript + jQuery The Next CEO of Stack OverflowGoogle Weather API wrapperOptimize JQuery/JavaScript Link CheckerInterupt redirect to catch potential newsletter signupJavaScript Weather AppJavaScript API for RESTWeather app using the openweathermap.com APIUsing JavaScript promises to display weather and location informationWeather API implementationDisplay Partner information JavaScript file using jQueryHandling keyup events with throttle or debounce in Javascript/jQuery
% symbol leads to superlong (forever?) compilations
How to make a variable always equal to the result of some calculations?
Does it take more energy to get to Venus or to Mars?
How to safely derail a train during transit?
What happens if you roll doubles 3 times then land on "Go to jail?"
What is meant by a M next to a roman numeral?
Return of the Riley Riddles in Reverse
Term for the "extreme-extension" version of a straw man fallacy?
What is the purpose of the Potent Cantrip ability?
Apart from "berlinern", do any other German dialects have a corresponding verb?
Subdividing a Mask
Customer Requests (Sometimes) Drive Me Bonkers!
Why use "finir par" instead of "finir de" before an infinitive?
What makes a siege story/plot interesting?
How can a function with a hole (removable discontinuity) equal a function with no hole?
What is the difference between "behavior" and "behaviour"?
Are there languages with no euphemisms?
suction cup thing with 1/4 TRS cable?
Overlapping nodes in a decision tree
How long to clear the 'suck zone' of a turbofan after start is initiated?
MAZDA 3 2006 (UK) - poor acceleration then takes off at 3250 revs
Why do remote companies require working in the US?
How do I construct this japanese bowl?
Trouble understanding the speech of overseas colleagues
Use weather API to get information in JavaScript + jQuery
The Next CEO of Stack OverflowGoogle Weather API wrapperOptimize JQuery/JavaScript Link CheckerInterupt redirect to catch potential newsletter signupJavaScript Weather AppJavaScript API for RESTWeather app using the openweathermap.com APIUsing JavaScript promises to display weather and location informationWeather API implementationDisplay Partner information JavaScript file using jQueryHandling keyup events with throttle or debounce in Javascript/jQuery
$begingroup$
Please can any code experts review and improve my code?
HTML
<!DOCTYPE html>
<html>
<head>
<title>API</title>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=font1|font2|etc" type="text/css">
<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
<h1 id="test"></h1>
<div class="weather-container">
<img id="icon">
<h1 id="weather"></h1>
<p>Description: <span id="desc"></span></p>
<p>Temperature: <span id="temp"></span></p>
</div>
<script src="mousetrap.js"></script>
<script src="moment.js"></script>
<script src="jquery.js"></script>
<script src="script.js"></script>
</body>
</html>
That's the HTML. It is just mostly tags with id to output info.
JavaScript
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
$.getJSON("https://api.openweathermap.org/data/2.5/weather?q="+ask+"&units=metric&appid=32b8cd17f2ff5d84d72342dd7408bab2", function(data)
console.log(data);
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
);
If there is any other language than jQquery that I can use to get JSON files from API, please let me know because I find jQuery quite confusing.
javascript jquery api html5 ajax
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
$endgroup$
bumped to the homepage by Community♦ 16 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
add a comment |
$begingroup$
Please can any code experts review and improve my code?
HTML
<!DOCTYPE html>
<html>
<head>
<title>API</title>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=font1|font2|etc" type="text/css">
<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
<h1 id="test"></h1>
<div class="weather-container">
<img id="icon">
<h1 id="weather"></h1>
<p>Description: <span id="desc"></span></p>
<p>Temperature: <span id="temp"></span></p>
</div>
<script src="mousetrap.js"></script>
<script src="moment.js"></script>
<script src="jquery.js"></script>
<script src="script.js"></script>
</body>
</html>
That's the HTML. It is just mostly tags with id to output info.
JavaScript
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
$.getJSON("https://api.openweathermap.org/data/2.5/weather?q="+ask+"&units=metric&appid=32b8cd17f2ff5d84d72342dd7408bab2", function(data)
console.log(data);
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
);
If there is any other language than jQquery that I can use to get JSON files from API, please let me know because I find jQuery quite confusing.
javascript jquery api html5 ajax
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
$endgroup$
bumped to the homepage by Community♦ 16 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08
add a comment |
$begingroup$
Please can any code experts review and improve my code?
HTML
<!DOCTYPE html>
<html>
<head>
<title>API</title>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=font1|font2|etc" type="text/css">
<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
<h1 id="test"></h1>
<div class="weather-container">
<img id="icon">
<h1 id="weather"></h1>
<p>Description: <span id="desc"></span></p>
<p>Temperature: <span id="temp"></span></p>
</div>
<script src="mousetrap.js"></script>
<script src="moment.js"></script>
<script src="jquery.js"></script>
<script src="script.js"></script>
</body>
</html>
That's the HTML. It is just mostly tags with id to output info.
JavaScript
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
$.getJSON("https://api.openweathermap.org/data/2.5/weather?q="+ask+"&units=metric&appid=32b8cd17f2ff5d84d72342dd7408bab2", function(data)
console.log(data);
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
);
If there is any other language than jQquery that I can use to get JSON files from API, please let me know because I find jQuery quite confusing.
javascript jquery api html5 ajax
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
$endgroup$
Please can any code experts review and improve my code?
HTML
<!DOCTYPE html>
<html>
<head>
<title>API</title>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
<link rel="stylesheet" href="//fonts.googleapis.com/css?family=font1|font2|etc" type="text/css">
<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
<h1 id="test"></h1>
<div class="weather-container">
<img id="icon">
<h1 id="weather"></h1>
<p>Description: <span id="desc"></span></p>
<p>Temperature: <span id="temp"></span></p>
</div>
<script src="mousetrap.js"></script>
<script src="moment.js"></script>
<script src="jquery.js"></script>
<script src="script.js"></script>
</body>
</html>
That's the HTML. It is just mostly tags with id to output info.
JavaScript
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
$.getJSON("https://api.openweathermap.org/data/2.5/weather?q="+ask+"&units=metric&appid=32b8cd17f2ff5d84d72342dd7408bab2", function(data)
console.log(data);
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
);
If there is any other language than jQquery that I can use to get JSON files from API, please let me know because I find jQuery quite confusing.
javascript jquery api html5 ajax
javascript jquery api html5 ajax
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
edited Feb 26 at 6:40
Jamal♦
30.4k11121227
30.4k11121227
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
asked Feb 23 at 17:05
XxxLegendxxXXxxLegendxxX
112
112
bumped to the homepage by Community♦ 16 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
bumped to the homepage by Community♦ 16 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08
add a comment |
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
Variables
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
None of these variables are really needed. Each variable is only used once and you aren't doing any incredibly complex calculations on them, so they aren't even needed for the sake of understanding. Eliminating variables would also get rid of this temp1 variable which is frankly a little ugly -- usually you know you've gone too far when you have to start adding numbers to variable names.
Try something like this instead:
$("#icon").attr("src", "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";);
document.getElementById('weather').innerHTML = data.weather[0].main;
document.getElementById('desc').innerHTML = data.weather[0].description;
document.getElementById('temp').innerHTML = data.main.temp + "℃";
No variables, same functionality, just as understandable.
You don't even need the ask variable, but I can understand why you're using it because it is indeed a long question.
jQuery
jQuery is not very relevant in today's JavaScript. I would recommend learning about fetch and thus Promises. This design is much cleaner and much nicer to work with, IMO.
Your code with fetch would look like this:
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
fetch(*url*).then(r => r.json()).then(data =>
...
);
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
$endgroup$
add a comment |
$begingroup$
Response to your question
If there is any other language than Jquery that I can use to get json files from api, please let me know because i find Jquery quite confusing :-).
As was mentioned in a comment (as well as a link-only answer that has since been deleted), there are alternatives, including but not limited to:
- vanilla JavaScipt:
- with
XMLHttpRequest- refer to UsingXMLHttpRequeston MDN - with the fetch API
- with
- other libraries, some of which are listed on youmightnotneedjquery.com/:
- reqwest
- then-request
- superagent
General review
User experience
You didn't state whether the specification was part of an assignment/task or just something you came up with, but the UX might be improved by having the prompt and Ajax request triggered by an event like a mouse click instead of page load.
If such a change was implemented, then it would be wise to cache the DOM references in variables instead of querying the DOM each time- e.g.
var weatherEl = document.getElementById('weather');
var descEl = document.getElementById('desc');
var tempEl = document.getElementById('temp');
//function to prompt
//AJAX response callback
function ajaxResponse(data)
//parse data
//...
weatherEl.innerHTML = data.weather[0].description;
descEl = data.weather[0].description;
tempEl.innerHTML = data.main.temp + "°C";
That way, the lines can be shorter, the DOM won't get queried each time, and you can likely eliminate those variables that are assigned and then only used once.
Perhaps a template system would help improve the process of updating the DOM elements instead of having to update each item individually.
Handling other responses and errors
The code above doesn't appear to (gracefully) handle any response other than a successful call to the API endpoint. When the user types in a city that the API doesn't recognize, then a 404 response is returned. If you continue to use the jQuery library, then a .fail() callback could be specified (see the $.getJSON() documentation for an example to handle that. Also, if a server error occurred, then a 5xx response might be returned.
Additionally, the code assumes that data.weather will be an array and have at least 1 element. What happens if either of those are not true? While it may seem implausible it may be possible and something to guard against.
Extra libraries
The code includes mousetrap.js and moment.js - I presume those correspond to the libraries Mousetrap and MomentJS, but it doesn't appear that those libraries are used by the code. Unless those are used by other code not included, those libraries can be removed to save the users time loading the page.
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f214121%2fuse-weather-api-to-get-information-in-javascript-jquery%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Variables
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
None of these variables are really needed. Each variable is only used once and you aren't doing any incredibly complex calculations on them, so they aren't even needed for the sake of understanding. Eliminating variables would also get rid of this temp1 variable which is frankly a little ugly -- usually you know you've gone too far when you have to start adding numbers to variable names.
Try something like this instead:
$("#icon").attr("src", "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";);
document.getElementById('weather').innerHTML = data.weather[0].main;
document.getElementById('desc').innerHTML = data.weather[0].description;
document.getElementById('temp').innerHTML = data.main.temp + "℃";
No variables, same functionality, just as understandable.
You don't even need the ask variable, but I can understand why you're using it because it is indeed a long question.
jQuery
jQuery is not very relevant in today's JavaScript. I would recommend learning about fetch and thus Promises. This design is much cleaner and much nicer to work with, IMO.
Your code with fetch would look like this:
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
fetch(*url*).then(r => r.json()).then(data =>
...
);
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
$endgroup$
add a comment |
$begingroup$
Variables
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
None of these variables are really needed. Each variable is only used once and you aren't doing any incredibly complex calculations on them, so they aren't even needed for the sake of understanding. Eliminating variables would also get rid of this temp1 variable which is frankly a little ugly -- usually you know you've gone too far when you have to start adding numbers to variable names.
Try something like this instead:
$("#icon").attr("src", "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";);
document.getElementById('weather').innerHTML = data.weather[0].main;
document.getElementById('desc').innerHTML = data.weather[0].description;
document.getElementById('temp').innerHTML = data.main.temp + "℃";
No variables, same functionality, just as understandable.
You don't even need the ask variable, but I can understand why you're using it because it is indeed a long question.
jQuery
jQuery is not very relevant in today's JavaScript. I would recommend learning about fetch and thus Promises. This design is much cleaner and much nicer to work with, IMO.
Your code with fetch would look like this:
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
fetch(*url*).then(r => r.json()).then(data =>
...
);
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
$endgroup$
add a comment |
$begingroup$
Variables
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
None of these variables are really needed. Each variable is only used once and you aren't doing any incredibly complex calculations on them, so they aren't even needed for the sake of understanding. Eliminating variables would also get rid of this temp1 variable which is frankly a little ugly -- usually you know you've gone too far when you have to start adding numbers to variable names.
Try something like this instead:
$("#icon").attr("src", "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";);
document.getElementById('weather').innerHTML = data.weather[0].main;
document.getElementById('desc').innerHTML = data.weather[0].description;
document.getElementById('temp').innerHTML = data.main.temp + "℃";
No variables, same functionality, just as understandable.
You don't even need the ask variable, but I can understand why you're using it because it is indeed a long question.
jQuery
jQuery is not very relevant in today's JavaScript. I would recommend learning about fetch and thus Promises. This design is much cleaner and much nicer to work with, IMO.
Your code with fetch would look like this:
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
fetch(*url*).then(r => r.json()).then(data =>
...
);
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
$endgroup$
Variables
var icon = "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";
var weather = data.weather[0].main;
var desc = data.weather[0].description;
var temp = data.main.temp;
var temp1 = temp + "℃"
$("#icon").attr("src", icon);
document.getElementById('weather').innerHTML = weather;
document.getElementById('desc').innerHTML = desc;
document.getElementById('temp').innerHTML = temp1;
None of these variables are really needed. Each variable is only used once and you aren't doing any incredibly complex calculations on them, so they aren't even needed for the sake of understanding. Eliminating variables would also get rid of this temp1 variable which is frankly a little ugly -- usually you know you've gone too far when you have to start adding numbers to variable names.
Try something like this instead:
$("#icon").attr("src", "https://openweathermap.org/img/w/" + data.weather[0].icon + ".png";);
document.getElementById('weather').innerHTML = data.weather[0].main;
document.getElementById('desc').innerHTML = data.weather[0].description;
document.getElementById('temp').innerHTML = data.main.temp + "℃";
No variables, same functionality, just as understandable.
You don't even need the ask variable, but I can understand why you're using it because it is indeed a long question.
jQuery
jQuery is not very relevant in today's JavaScript. I would recommend learning about fetch and thus Promises. This design is much cleaner and much nicer to work with, IMO.
Your code with fetch would look like this:
var ask = prompt("Type in your city or town that you want the weather for. Please make sure you write the first letter as capital letter and you spell it right.");
fetch(*url*).then(r => r.json()).then(data =>
...
);
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
edited Feb 26 at 16:34
Sᴀᴍ Onᴇᴌᴀ
9,92162166
9,92162166
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
answered Feb 25 at 22:29
SirPythonSirPython
12k32891
12k32891
add a comment |
add a comment |
$begingroup$
Response to your question
If there is any other language than Jquery that I can use to get json files from api, please let me know because i find Jquery quite confusing :-).
As was mentioned in a comment (as well as a link-only answer that has since been deleted), there are alternatives, including but not limited to:
- vanilla JavaScipt:
- with
XMLHttpRequest- refer to UsingXMLHttpRequeston MDN - with the fetch API
- with
- other libraries, some of which are listed on youmightnotneedjquery.com/:
- reqwest
- then-request
- superagent
General review
User experience
You didn't state whether the specification was part of an assignment/task or just something you came up with, but the UX might be improved by having the prompt and Ajax request triggered by an event like a mouse click instead of page load.
If such a change was implemented, then it would be wise to cache the DOM references in variables instead of querying the DOM each time- e.g.
var weatherEl = document.getElementById('weather');
var descEl = document.getElementById('desc');
var tempEl = document.getElementById('temp');
//function to prompt
//AJAX response callback
function ajaxResponse(data)
//parse data
//...
weatherEl.innerHTML = data.weather[0].description;
descEl = data.weather[0].description;
tempEl.innerHTML = data.main.temp + "°C";
That way, the lines can be shorter, the DOM won't get queried each time, and you can likely eliminate those variables that are assigned and then only used once.
Perhaps a template system would help improve the process of updating the DOM elements instead of having to update each item individually.
Handling other responses and errors
The code above doesn't appear to (gracefully) handle any response other than a successful call to the API endpoint. When the user types in a city that the API doesn't recognize, then a 404 response is returned. If you continue to use the jQuery library, then a .fail() callback could be specified (see the $.getJSON() documentation for an example to handle that. Also, if a server error occurred, then a 5xx response might be returned.
Additionally, the code assumes that data.weather will be an array and have at least 1 element. What happens if either of those are not true? While it may seem implausible it may be possible and something to guard against.
Extra libraries
The code includes mousetrap.js and moment.js - I presume those correspond to the libraries Mousetrap and MomentJS, but it doesn't appear that those libraries are used by the code. Unless those are used by other code not included, those libraries can be removed to save the users time loading the page.
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
$endgroup$
add a comment |
$begingroup$
Response to your question
If there is any other language than Jquery that I can use to get json files from api, please let me know because i find Jquery quite confusing :-).
As was mentioned in a comment (as well as a link-only answer that has since been deleted), there are alternatives, including but not limited to:
- vanilla JavaScipt:
- with
XMLHttpRequest- refer to UsingXMLHttpRequeston MDN - with the fetch API
- with
- other libraries, some of which are listed on youmightnotneedjquery.com/:
- reqwest
- then-request
- superagent
General review
User experience
You didn't state whether the specification was part of an assignment/task or just something you came up with, but the UX might be improved by having the prompt and Ajax request triggered by an event like a mouse click instead of page load.
If such a change was implemented, then it would be wise to cache the DOM references in variables instead of querying the DOM each time- e.g.
var weatherEl = document.getElementById('weather');
var descEl = document.getElementById('desc');
var tempEl = document.getElementById('temp');
//function to prompt
//AJAX response callback
function ajaxResponse(data)
//parse data
//...
weatherEl.innerHTML = data.weather[0].description;
descEl = data.weather[0].description;
tempEl.innerHTML = data.main.temp + "°C";
That way, the lines can be shorter, the DOM won't get queried each time, and you can likely eliminate those variables that are assigned and then only used once.
Perhaps a template system would help improve the process of updating the DOM elements instead of having to update each item individually.
Handling other responses and errors
The code above doesn't appear to (gracefully) handle any response other than a successful call to the API endpoint. When the user types in a city that the API doesn't recognize, then a 404 response is returned. If you continue to use the jQuery library, then a .fail() callback could be specified (see the $.getJSON() documentation for an example to handle that. Also, if a server error occurred, then a 5xx response might be returned.
Additionally, the code assumes that data.weather will be an array and have at least 1 element. What happens if either of those are not true? While it may seem implausible it may be possible and something to guard against.
Extra libraries
The code includes mousetrap.js and moment.js - I presume those correspond to the libraries Mousetrap and MomentJS, but it doesn't appear that those libraries are used by the code. Unless those are used by other code not included, those libraries can be removed to save the users time loading the page.
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
$endgroup$
add a comment |
$begingroup$
Response to your question
If there is any other language than Jquery that I can use to get json files from api, please let me know because i find Jquery quite confusing :-).
As was mentioned in a comment (as well as a link-only answer that has since been deleted), there are alternatives, including but not limited to:
- vanilla JavaScipt:
- with
XMLHttpRequest- refer to UsingXMLHttpRequeston MDN - with the fetch API
- with
- other libraries, some of which are listed on youmightnotneedjquery.com/:
- reqwest
- then-request
- superagent
General review
User experience
You didn't state whether the specification was part of an assignment/task or just something you came up with, but the UX might be improved by having the prompt and Ajax request triggered by an event like a mouse click instead of page load.
If such a change was implemented, then it would be wise to cache the DOM references in variables instead of querying the DOM each time- e.g.
var weatherEl = document.getElementById('weather');
var descEl = document.getElementById('desc');
var tempEl = document.getElementById('temp');
//function to prompt
//AJAX response callback
function ajaxResponse(data)
//parse data
//...
weatherEl.innerHTML = data.weather[0].description;
descEl = data.weather[0].description;
tempEl.innerHTML = data.main.temp + "°C";
That way, the lines can be shorter, the DOM won't get queried each time, and you can likely eliminate those variables that are assigned and then only used once.
Perhaps a template system would help improve the process of updating the DOM elements instead of having to update each item individually.
Handling other responses and errors
The code above doesn't appear to (gracefully) handle any response other than a successful call to the API endpoint. When the user types in a city that the API doesn't recognize, then a 404 response is returned. If you continue to use the jQuery library, then a .fail() callback could be specified (see the $.getJSON() documentation for an example to handle that. Also, if a server error occurred, then a 5xx response might be returned.
Additionally, the code assumes that data.weather will be an array and have at least 1 element. What happens if either of those are not true? While it may seem implausible it may be possible and something to guard against.
Extra libraries
The code includes mousetrap.js and moment.js - I presume those correspond to the libraries Mousetrap and MomentJS, but it doesn't appear that those libraries are used by the code. Unless those are used by other code not included, those libraries can be removed to save the users time loading the page.
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
$endgroup$
Response to your question
If there is any other language than Jquery that I can use to get json files from api, please let me know because i find Jquery quite confusing :-).
As was mentioned in a comment (as well as a link-only answer that has since been deleted), there are alternatives, including but not limited to:
- vanilla JavaScipt:
- with
XMLHttpRequest- refer to UsingXMLHttpRequeston MDN - with the fetch API
- with
- other libraries, some of which are listed on youmightnotneedjquery.com/:
- reqwest
- then-request
- superagent
General review
User experience
You didn't state whether the specification was part of an assignment/task or just something you came up with, but the UX might be improved by having the prompt and Ajax request triggered by an event like a mouse click instead of page load.
If such a change was implemented, then it would be wise to cache the DOM references in variables instead of querying the DOM each time- e.g.
var weatherEl = document.getElementById('weather');
var descEl = document.getElementById('desc');
var tempEl = document.getElementById('temp');
//function to prompt
//AJAX response callback
function ajaxResponse(data)
//parse data
//...
weatherEl.innerHTML = data.weather[0].description;
descEl = data.weather[0].description;
tempEl.innerHTML = data.main.temp + "°C";
That way, the lines can be shorter, the DOM won't get queried each time, and you can likely eliminate those variables that are assigned and then only used once.
Perhaps a template system would help improve the process of updating the DOM elements instead of having to update each item individually.
Handling other responses and errors
The code above doesn't appear to (gracefully) handle any response other than a successful call to the API endpoint. When the user types in a city that the API doesn't recognize, then a 404 response is returned. If you continue to use the jQuery library, then a .fail() callback could be specified (see the $.getJSON() documentation for an example to handle that. Also, if a server error occurred, then a 5xx response might be returned.
Additionally, the code assumes that data.weather will be an array and have at least 1 element. What happens if either of those are not true? While it may seem implausible it may be possible and something to guard against.
Extra libraries
The code includes mousetrap.js and moment.js - I presume those correspond to the libraries Mousetrap and MomentJS, but it doesn't appear that those libraries are used by the code. Unless those are used by other code not included, those libraries can be removed to save the users time loading the page.
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
edited Feb 26 at 16:35
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
answered Feb 25 at 20:59
Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ
9,92162166
9,92162166
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f214121%2fuse-weather-api-to-get-information-in-javascript-jquery%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e)
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom))
StackExchange.using('gps', function() StackExchange.gps.track('embedded_signup_form.view', location: 'question_page' ); );
$window.unbind('scroll', onScroll);
;
$window.on('scroll', onScroll);
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
Hello welcome to CR. This looks interesting. If there is any other language than Jquery -> well jQuery is not a language it is a library. There are various ways to call APIs see here
$endgroup$
– 422_unprocessable_entity
Feb 23 at 17:20
$begingroup$
Welcome to Code Review! You can get a line break without a blank line: just append two blanks at the end of the line that shall be followed by the break.
$endgroup$
– greybeard
Feb 23 at 23:08