Optimizing JSON append template

Question

I am fetching a list of messages from some JSON and appending each message as an LI within a UL. I have created the message template within jQuery to append each message to the document. My code works well but I can't help but feel it could be improved (I am a beginner JS programmer). From what I have seen online moving the templating to handlebars.js would be a better solution but I can't be sure.

$(document).ready(function() {
 var broadcastMessagesJsonURL = "https://api.myjson.com/bins/sadap";
 $.getJSON(broadcastMessagesJsonURL, function(data) {
 $.each(data.broadcastMessages, function(i, item) {
 //All items from the json
 var broadcastMessageID = (item.ID);
 var checkBroadcastMessageRead = (item.read ? " broadcast__message__read__state--unread" : "");
 var broadcastMessageSubject = (item.subject);
 var broadcastMessageGroup = (item.group);
 var broadcastMessageDateSent = (item.dateSent);
 var checkBroadcastFeatureImage = (item.featureImage ? " broadcast__message__image--active" : "");
 var checkBroadcastForm = (item.form ? " broadcast__message__form--active" : "");
 var checkBroadcastAttachments = (item.attachments ? " broadcast__message__attachment--active" : "");
 var broadcastMessageContent = (item.content);
 var broadcastMessageTemplate = ('<li class="broadcast__message__list__item" data-broadcast-message-ID="' + broadcastMessageID + '"> \
 <div class="broadcast__message__wrapper"> \
 <div class="broadcast__message__read__state' + checkBroadcastMessageRead + '"></div> \
 <div class="broadcast__message__subject">' + broadcastMessageSubject + '</div> \
 <div class="broadcast__message__group">' + broadcastMessageGroup + '</div> \
 <div class="broadcast__message__date__time__stamp" title="' + broadcastMessageDateSent + '">' + broadcastMessageDateSent + '</div> \
 <div class="broadcast__message__snippet">' + broadcastMessageContent + '</div> \
 </div> \
 </li>');
 $(".broadcast__messages__list").append(broadcastMessageTemplate);
});
 }).fail(function() {
 console.log("broadcastMessages json cannot be loaded");
 });
});

CODEPEN LINK

Answer 1

Rewrite

Below is a rewrite with all my further suggestions applied.

const messagesURL = 'https://api.myjson.com/bins/sadap';
const messagesLoadError = () => console.error('broadcastMessages json cannot be loaded');
const forEachMessage = (_, message) => {
 const messageRead = message.read ? ' broadcast__message__read__state--unread' : '',
 featureImage = message.featureImage ? ' broadcast__message__image--active' : '',
 form = message.form ? ' broadcast__message__form--active' : '',
 attachments = message.attachments ? ' broadcast__message__attachment--active' : '';
 const template = `<li class="broadcast__message__list__message" data-broadcast-message-ID="${message.ID}">
 <div class="broadcast__message__wrapper">
 <div class="broadcast__message__read__state${messageRead}"></div>
 <div class="broadcast__message__subject">${message.subject}</div>
 <div class="broadcast__message__group">${message.group}</div>
 <div class="broadcast__message__date__time__stamp" title="${message.dateSent}">${message.dateSent}</div>
 <div class="broadcast__message__snippet">${message.content}</div>
 </div>
 </li>`;
 $('.broadcast__messages__list').append(template);
};
$.getJSON(messagesURL, data => $.each(data.broadcastMessages, forEachMessage)).fail(messagesLoadError);

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"
 integrity="sha384-fj9YEHKNa/e0CNquG4NcocjoyMATYo1k2Ff5wGB42C/9AwOlJjDoySPtNJalccfI"
 crossorigin="anonymous">
</script>

Remarks

API security

Your API endpoint (https://api.myjson.com/bins/sadap) uses insecure and archaic SSL configuration. Actually, I couldn't connect to it at all, since I have tweaked up browser configuration. Few key points about your API's security:

• Vulnerable to the OpenSSL CCS vulnerability (CVE-2014-0224),
• Relies fully on insecure RC4 or legacy CBC mode of ciphers it uses,
• Doesn't support Forward Secrecy.

See Qualys' test result.

Unused callback's parameter

You have the following line in your code:

$.each(data.broadcastMessages, function(i, item) {

but i is never used. In such cases, it's good to replace it with underscore (_).

Naming issues

As Leon Bambrick added on top of Phil Karlton's quote:

There are only two hard things in Computer Science: cache invalidation, naming things, and off-by-one errors.

„Some" of your names are overly verbose (word broadcast seem to be everywhere) and other are not descriptive enough, like item.

Wall of text

This

var broadcastMessageID = (item.ID);
var checkBroadcastMessageRead = (item.read ? " broadcast__message__read__state--unread" : "");
var broadcastMessageSubject = (item.subject);
var broadcastMessageGroup = (item.group);
var broadcastMessageDateSent = (item.dateSent);
var checkBroadcastFeatureImage = (item.featureImage ? " broadcast__message__image--active" : "");
var checkBroadcastForm = (item.form ? " broadcast__message__form--active" : "");
var checkBroadcastAttachments = (item.attachments ? " broadcast__message__attachment--active" : "");
var broadcastMessageContent = (item.content);

would be much easier to read, if you would combine vars and align equality signs, question marks and colons:

var broadcastMessageID = item.ID,
 broadcastMessageSubject = item.subject,
 broadcastMessageGroup = item.group,
 broadcastMessageDateSent = item.dateSent,
 broadcastMessageContent = item.content,
 checkBroadcastMessageRead = item.read ? ' broadcast__message__read__state--unread' : '',
 checkBroadcastFeatureImage = item.featureImage ? ' broadcast__message__image--active' : '',
 checkBroadcastForm = item.form ? ' broadcast__message__form--active' : '',
 checkBroadcastAttachments = item.attachments ? ' broadcast__message__attachment--active' : '';

Variables

Most of your variables are unnecessary. If you would rename ambiguous item to message, you could use message.ID instead of doing var broadcastMessageID = (item.ID).

Also, bear in mind that the following three variables are unused: checkBroadcastFeatureImage, checkBroadcastForm, checkBroadcastAttachments.

Use template literals

var broadcastMessageTemplate could become much more easily readable if you used template literals to declare it. Mind however, that this is still not the „most vanilla" method to create DOM elements either. Take a look at createElement().

Fail notification

You should use console.error() instead of console.log() in your fail's callback.

• Joseph: Pro tip from a person who used to align variable names and ternaries - they're readable, true. But they're a hassle to maintain, especially when another, longer-named variable comes in. Commits become noisy because of alignment adjustments.
• 2ne: Thank you for an excellent answer! The only part that I don't really understand is but i is never used. In such cases, it's good to replace it with underscore (_). Why would I still need the (_)?
• Przemek: @2ne: Because in your callback where i appears, you need to access only it's second parameter. As such, you must name somehow all the preceding parameters, as well. The first one is never actually used, so by convention it should be named _.
• Przemek: @JosephtheDreamer: Somehow valid, although I think that in this case pros would top the cons. But it's a matter of personal preference, of course.