Show the Local Weather, please help me make it more functional?

JavaScript
1

Hey, I have just finished this project, with some huge help already, pointing me in the right direction. Much appreciated and I do understand certain aspects a lot better, but I feel im not there yet, enough to progress further. My codes works but I want to do things with it, things I dont know how, to make it better and more functional. Please help me make my temperature conversion into a function on its own, so I can call it. I know my code is a mess, and I really want to start in the right direction on functioning my code, making things easy to see and use.

//decalre some global variables
var api = "https://fcc-weather-api.glitch.me/api/current?lat=";
var lat;
var long;
var currentTemp;
var far;

$(document).ready(function() { //on document load
  if (navigator.geolocation) { //if geolocation allowed
    navigator.geolocation.getCurrentPosition(function(position) { 
      var lat = position.coords.latitude; //store lat position
      var long = position.coords.longitude; //store long position
      getInfo(lat, long); //call getInfo function
    });
  } else {
    console.log("geolocation not supported"); //if not supported
  }
});
//function to get all the data and use it on the screen
function getInfo(lat, long) { //pass lat and long thrugh function
  var urlstring = api + lat + "&&lon=" + long; //decalre url variable to get location
  $.ajax({
    url: urlstring, //use this url
    success: function(data) { //successfull
      var icon = '"' + data.weather[0].icon + '"'; //this is the built in icons on API
      var overView = data.weather[0].main; //overview from object
      var desc = data.weather[0].description; //decirption
      //needed to use 2 variables. One for rounded and one for working out far conversion. Kept getting NAN when trying to use currentTemp rounded with Far
      var currentTemp = data.main.temp;
      var currentTempRounded = Math.round(currentTemp) + String.fromCharCode(176) + "C";
      var far = currentTemp * (9 / 5) + 32;
      var farRounded = Math.round(far) + String.fromCharCode(176) + "F";
      var name = data.name;
      //Put data in span locations....
      $("#icon").append("<image src=" + icon + "</image>");
      $("#location").text(name);
      $("#temp").text(currentTempRounded);
      $("#over").text(overView);
      $("#script").text(desc);
      //I REALLY want to make this its own function!! Outside of getInfo();
      //litte on click function, if Celsius rounded change to far rounded
      //else switch to celsius
$("#temp").click(function() {
  if ($("#temp").text() == currentTempRounded){
  $("#temp").text(farRounded);}
  else{
    $("#temp").text(currentTempRounded);
  }
 });
    }
  });
}

WIll also attach my Codepen, its enough to pass the problem, but to me, could be so much better, not so much what it does, how im coding it.

Thanks

3

Most of your commenting is redundant. The only comments that potentially add clarity are:

//function to get all the data and use it on the screen
//this is the built in icons on API
//needed to use 2 variables. One for rounded and one for working out far conversion. Kept getting NAN when trying to use currentTemp rounded with Far

Even then, the first of those would be better as a more descriptive function name (getAndPrintInfo() or similar). The fact that there’s an “and” in that name also suggests it should probably be two functions, getInfo() and populateUI() or something. Then, in your document ready handler, you could have something like the following:

var weatherInfo = getInfo(lat, long); //getInfo returns an object with all the data you need
populateUI(weatherInfo); //populateUI uses the data from the returned object

In fact, looking at things like commenting and naming can often reveal ways to further modularize your code like this.

Regarding when you should comment, this is a good article:

https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/

4

Thanks for the feedback, I will make some improvements!