Thinking in OOP - Local Weather Project

Thinking in OOP - Local Weather Project
0

#1

I have trouble thinking in OOP. I see the good programmers separate everything into smaller functions, but every time I try, I get hung up for a while. My weather page is loading, but there are certain things I wanted to separate to make it more clear and formatted. Like I have my jQuery .appends in the getData function, yet I think they should be separated. I would think I would want to access just a data object from the main block and just use the jQuery appends in a different part of the page. Maybe I am overthinking. I am not even sure what the document.ready function is for. I had all my code in it prior, but when I took it out. It still works. I guess my question is what’s the thought process of best practices programming for separating different parts and functions of a web page. Any advice or tips would be great!~ ^^

navigator.geolocation.getCurrentPosition(function(pos) {
  load(pos.coords.latitude, pos.coords.longitude);
});

function getData(data) {
  var city =
    "<h2 class='city'>" + data.name + ", " + data.sys.country + "</h2>";
  var temp =
    "<h2 class='temp'>" +
    data.main.temp +
    "&deg <a href='#' id='convert'>C</a></h2>";
  var sky = "<h2 class='sky'>" + data.weather[0].main + "</h2>";
  var img = "<img align='middle' src=" + data.weather[0].icon + ">";

  $(".weather").append(city);
  $(".weather").append(temp);
  $(".weather").append(sky);
  $(".weather").append(img);
}

function load(lat, lon) {
  var api =
    "https://fcc-weather-api.glitch.me/api/current?lat=" + lat + "&lon=" + lon;
  $.getJSON(api, getData);
}

$(document).ready(function() {

});

#2

I think you’re misunderstanding OOP and code separation/refactoring, or at least misusing the terms. OOP isn’t just about separating code out into smaller pieces; typically people talk about classes, inheritance, and such where they are creating many objects by reusing the same pattern.

In terms of refactoring code, it’s kind of hard for this particular project, because there aren’t many parts of the project that are repeating (if any). If things are repeating, then it makes sense to make it into its own function, however not everything needs to be in it’s own function, or soon it’s just code with everything in a separate function. I guess unless you know that something needs to be its own function, don’t separate it out, because premature refactoring will be a waste of time. Leaving good comments is also a good practice for others (or yourself)- this isn’t every line but is a good reminder why you did a certain thing/wrote the code that way.

$(document).ready is used to ensure that the DOM is loaded before JavaScript runs. It’s like telling the code within it, “hey, we’re waiting for the page to load before running this JS inside this function.” Without it the JavaScript runs immediately. If you put the <script> tags at the bottom of your page before the </body>, you should be fine.


#3

That clears up a lot of things for me. I feel better that you are saying refactoring is out of the scope of this project. Thanks for replying!


#4

I actually like your idea of separating the code which builds and displays the html from that of
getting the data.

Also, I would put your call to navigator.geolocation.getCurrentPosition inside the $(document).ready function.

Below is one possible way to separate getting the data from displaying the data. The displayWeather function would take care of updating the page’s div with class=“weather”. You will notice instead of having 4 separate calls to append, I built the html as a string and used the html function one time. This is better, because you only re-render the page one time instead of four times. Also, append is slower than the html function.

Like @v-lai said above, because there is not much going on in the project, this might be considered overkill, but it is good practice to make your functions small and try to perform a specific task.

function getData(data) {
  var location = data.name + ", " + data.sys.country;
  var temp = data.main.temp; 
  var sky = data.weather[0].main;
  var img = data.weather[0].icon;
  displayWeather(location, temp, sky, img);
}

function load(lat, lon) {
  var api = "https://fcc-weather-api.glitch.me/api/current?lat=" + lat + "&lon=" + lon;
  $.getJSON(api, getData);
}

function displayWeather(location, temp, sky, img) {
  var weatherHTML= "<h2 class='city'>" + location + "</h2>";
  html += "<h2 class='temp'>" +  temp  + "&deg <a href='#' id='convert'>C</a></h2>";
  html += "<h2 class='sky'>" + sky  + "</h2>";
  html += "<img align='middle' src=" + img + ">"
  $(".weather").html(weatherHTML);
}

$(document).ready(function() {
  navigator.geolocation.getCurrentPosition(function(pos) {
    load(pos.coords.latitude, pos.coords.longitude);
  });
});

If you use a Template Literal for the html variable, the code in the displayWeather function becomes much more readable.

function displayWeather(location, temp, sky, img) {
  var weatherHTML = `
    <h2 class='city'>${location}</h2>
    <h2 class='temp'>${temp}&deg<a href='#' id='convert'>C</a></h2>
    <h2 class='sky'>${sky}</h2>
    <img align='middle' src='${img}'> `;
  $('.weather').html(weatherHTML);
}

#5

@RandellDawson, this is perfect. Just what I was looking for separating the JSON objects from the HTML. I wasn’t aware about template literals. Thanks again!