When those other linting issues occurred, I was reminded that I haven’t properly formatted the code. What happened is that after wrapping single-line if statements with braces, that helps JS Beautifier to properly indent the code. But I hadn’t updated the indenting.
After running the code through JSBeautifier again the code is now all properly indented, and I have a nice long list of 10 lines that are still too long.
Taking care of long lines
jQuery("#selection").find(":selected").each(function(ignore, option) {
This one reminds me that I really need to go through the code removing that ugly patch of using ignore, and instead replace it with more appropriate toArray().forEach() instead. I’ll do that now.
// jQuery("#selection").find(":selected").each(function(ignore, option) {
jQuery("#selection").find(":selected").toArray().forEach(function(option) {
Now back to dealing with line length.
The jQuery is normally only used for the initial document ready wrapper. After that the $ symbol should be used instead.
jQuery(document).ready(function($) {
jQuery(document).ready(function($) {
var StatJSON = {
Hmm, that’s odd. Why does your code have multiple document ready wrappers? Removing those extra ones will definitely help with the long lines problem.
Also, renaming jQuery to $ everywhere inside of the document ready wrapper takes care of more long lines, leaving us only with 4 long lines.
Long line #1
The first long line is in the button submit handler.
$(".divResult table th:not(:first)").toArray().forEach(function(header) {
var columnHead = $(header).text().split(" ").join("");
$(header).addClass($(header).text().split(" ").join(""));
});
Here we can extract the query to a named variable:
var headings = $(".divResult table th:not(:first)");
headings.toArray().forEach(function(header) {
var columnHead = $(header).text().split(" ").join("");
$(header).addClass($(header).text().split(" ").join(""));
});
Long lines 2 through to 4
These are in the weather link code that we dealt with earlier:
if ($(td).text() === "Weather") {
$(td).nextAll("td").toArray().forEach(function(el) {
var path = $(el).text();
var linkHtml = `<a href="https://forecast7.com/en/${path}/"`;
linkHtml += ` class="weatherwidget-io" data-label_2="WEATHER"`;
linkHtml += ` data-days="3" data-theme="original">WEATHER</a>`;
$(el).html(linkHtml);
});
}
Here, while it is possible to shorten the linkHtml variable, a more appropriate way is to extract the looping function instead.
function addWeatherLink(el) {
var path = $(el).text();
var linkHtml = `<a href="https://forecast7.com/en/${path}/"`;
linkHtml += ` class="weatherwidget-io" data-label_2="WEATHER"`;
linkHtml += ` data-days="3" data-theme="original">WEATHER</a>`;
$(el).html(linkHtml);
}
$("#divResult table tbody tr td").toArray().forEach(function(td) {
if ($(td).text() === "Weather") {
$(td).nextAll("td").toArray().forEach(addWeatherLink);
}
__weatherwidget_init();
});
That is all of the too-long lines dealt with, and we can now move on to the rest of the linting.
Linting the rest of the code
Many of the linting issues are about using anonymous functions. We really need to name those. Something that I’ve been noticing about the code is that while it says what it does, the code doesn’t say why it does things. The first is necessary for the computer, but the second is really important for the human programmer.
That issue is remedied a lot by using good names for things.
One of the hardest parts of programming is finding good names for things. I can’t promise to find good names, but I’ll at least attempt to use meaningful names and seek to improve them where possible.
Naming the functions
Naming document ready function
For the document ready function it seems appropriate to name that one weatherTable, because it ranks the weather. I can’t help but find some measure of enjoyment too that rankWeather also works in terms of “bad weather” as well.
jQuery(document).ready(function rankWeather($) {
Naming a heading function
Here is how the code is before naming the function:
$.each(data, function(i) {
//getting heading at that key
html += "<th>" + StatJSON[data[i]].ColHeading + "</th>";
});
And afterwards the comment is found to be misleading.
$.each(data, function addColumnHeading(i) {
//getting heading at that key
html += "<th>" + StatJSON[data[i]].ColHeading + "</th>";
});
The heading isn’t being obtained. Instead it is being added as a table element. Because the function name provides enough of an understanding here now, the misleading comment should either be corrected, or removed. When comments are corrected usually that helps me to understand that the code is weak in that area, so I try to improve the code so that the comment isn’t needed. In this case the comment can be removed as it doesn’t add anything to what we know already.
$.each(data, function addColumnHeading(i) {
html += "<th>" + StatJSON[data[i]].ColHeading + "</th>";
});
Naming add table rows
This next part of the code needs naming too:
$.each(StatJSON[data[0]], function(k, v) {
html += "<tr>";
Here we are adding table rows, so addRow is a good function name.
The linter also tells me that v isn’t used, so that gets removed.
$.each(StatJSON[data[0]], function addRow(k) {
html += "<tr>";
I also tell myself that single letter variable names are almost completely useless. Here, k is the property names of each option in your StatJSON array. That is what they are, but we should give it a name for what they mean.
Here, they are the row headings, so that’s what we should name them.
$.each(StatJSON[data[0]], function addRow(rowTitle) {
html += "<tr>";
if (rowTitle !== "ColHeading") {
html += "<td>" + rowTitle + "</td>";
}
Naming add data function
The next function that needs naming is:
$.each(data, function(k2, v2) {
if (rowTitle !== "ColHeading") {
html += "<td>" + StatJSON[data[k2]][rowTitle] + "</td>";
}
});
Here it’s adding data from StatJSON, so calling it addData is good enough, especially in the context with the addRow function too.
$.each(data, function addData(k2, v2) {
if (rowTitle !== "ColHeading") {
html += "<td>" + StatJSON[data[k2]][rowTitle] + "</td>";
}
});
The linter tells me that v2 isn’t used, and I tell myself that k2 needs to be renamed to something better. While k2 is an option name such as “Option2”, that’s only what it is and doesn’t tell us why it’s important to us. Each of those options refers to info about a city, so cityInfo seems to be a good name to use there.
$.each(data, function addData(cityInfo) {
if (rowTitle !== "ColHeading") {
html += "<td>" + StatJSON[data[cityInfo]][rowTitle] + "</td>";
}
});
We now get told that the line is too long, so I’ll extract out a named variable from there.
$.each(data, function addData(cityInfo) {
if (rowTitle !== "ColHeading") {
var cityData = StatJSON[data[cityInfo]][rowTitle];
html += "<td>" + cityData + "</td>";
}
});
Naming button submit code
Here’s the next piece that the linter says needs a function name:
$("#btnSubmit").click(function() {
var data = [];
...
});
That function is an event handler for submitting the cities, so calling it citiesSubmitHandler is a good and standard way of naming it.
$("#btnSubmit").click(function citiesSubmitHandler() {
var data = [];
Now with event handlers, it’s best if they do very little work at all themself, and pass on execution to other functions. I’m tempted to add a TODO comment to the code, but the linter doesn’t allow anything else to be done until those TODO’s are taken out.
So, I guess that I had better move out the code now before it gets forgotten about.
And that is going to be the topic of my next post.
Find attached the .html file with the same code.
Learnt a lot there!!
