JSON call for stock info using YQL

Question

I got this code working but it feels clunky. I am wondering if there is a way I can improve it.

(I'm intermingling JavaScript and jQuery). I'm using jquery-1.11.1.min.js hosted on code.jquery.com.

Here is the jsFiddle

JS:

<script>
$('#indices > div').each(function () { 
var url = "http://query.yahooapis.com/v1/public/yql";
var $this = $(this);
var symbol = $this.attr('id');
var data = encodeURIComponent("select * from yahoo.finance.quotes where symbol in ('" + symbol + "')");
$.getJSON(url, 'q=' + data + "&format=json&diagnostics=true&env=http://datatables.org/alltables.env")
 .done(function (data) {
 $this.children('#aname').text(data.query.results.quote.Symbol);
 $this.children('#aresult').text(data.query.results.quote.LastTradePriceOnly);
 $this.children('#apercentchg').text(data.query.results.quote.PercentChange);
 $this.children('#adollarchg').text(data.query.results.quote.Change);
 if (data.query.results.quote.PercentChange.indexOf("+") != -1) {
 $this.addClass("divgreen");
 $this.children('#apercentchg').className = "greenText";
 } else {
 $this.addClass("divred");
 $this.children('#apercentchg').className = "redText";
 }
 });
});
</script>

HTML:

<div id="indices">
 <div id="^GSPC">
 <span>S&amp;P 500</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
 <div id="^NDX">
 <span>Nasdaq 100</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
 <div id="GLD">
 <span>SPDR Gold</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
 <div id="SLV">
 <span>iShares Silver</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
 <div id="^N225">
 <span>Nikkei 225</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
 <div id="^FTSE">
 <span>FTSE</span><br> 
 <span id="aresult"></span> <span id="adollarchg"></span> 
 </div>
</div>

CSS:

<style>
div.indices {
 font: 12px/19px Helvetica, Arial, sans-serif;
 width: 825px;
 margin: 0 auto;
}
.divgreen {
 color: #000;
 float: left;
 background-color: #70DB70;
 border-right: 3px solid #FFF;
 padding: 3px;
}
.divred {
 color: #000;
 float: left;
 background-color: #f7cdc2;
 border-right: 3px solid #FFF;
 padding: 3px;
}
</style>

Update Here is the final product of the great suggestions below and some additional changes. I also added a 'stretchy' ability that targets the parent container size and when that view able space gets to small to work with it hides the stock information bar. I did this to reduce clutter for mobile users.

http://jsfiddle.net/franktudor/LZm2f/

and it works like a charm and I hope someone else finds this useful for their website.

• Flambino: "intermingling JavaScript and jQuery". Well, anything else would be impossible. jQuery is written in JavaScript. JavaScript is the language; jQuery is just another library.

Answer 1

1. Don't repeat element IDs. What you want is a data-* attribute, e.g. data-property. Or you could use class names, since those can repeat and you'll want to style things anyway. Of course, you could also build your elements as needed.
2. Use CSS classes properly. If you've set the containing DIV's class, you just need to write your CSS to automatically style its descendants properly, instead of setting a class like redtext. While you're at it, don't use an element name in your class names. If you want target a specific tag a selector like div.green is what you need, in which case the class name would just be green. However, green is problematic too; you're not saying it should be green, you're saying that that index/stock is up or rising. So name your classes that; i.e. name them something semantically appropriate.
3. It'd be simpler to first collect all the symbol names, and then query Yahoo once for all of them, instead of firing off queries one at a time.
4. Use $.getJSON's ability to accept a data object, rather than manually creating a query string

For instance, you could use this HTML (repeat as needed, unless you want to simply build it in JS):

<div class="symbol" data-symbol="^GSPC">
 <span class="name">S&amp;P 500</span>
 <span class="result" data-property="LastTradePriceOnly"></span>
 <span class="change" data-property="Change"></span>
</div>

and this CSS:

.symbol { /* basic styling */ }
.symbol.up { /* styling for a symbol that's "up" */ }
.symbol.up .change { /* styling for the change span */ }
.symbol.down { /* styling for a symbol that's "down" */ }
.symbol.down .change { /* styling for the change span */ }

As for the JavaScript

var elements, symbols, query;
// get the elements
elements = $(".symbol");
// get the symbols and join them to a comma-separated string
symbols = elements.map(function () {
 var symbol = $(this).data("symbol")
 return "'"+symbol+"'";
}).toArray().join(",");
// get the quotes
$.getJSON("http://query.yahooapis.com/v1/public/yql", {
 format: "json",
 diagnostics: "true",
 env: "http://datatables.org/alltables.env",
 q: "select * from yahoo.finance.quotes where symbol in (" + symbols + ")"
}, function (data, xhr, status) {
 // TODO: sanity checking of the data
 data.query.results.quote.forEach(function (quote) {
 var element = elements.filter("[data-symbol='"+quote.Symbol+"']:first");
 if(/^\+/.test(quote.PercentChange)) {
 element.addClass("up");
 } else {
 element.addClass("down");
 }
 element.children("[data-property]").each(function () {
 var child = $(this),
 prop = child.data("property");
 child.text(quote[prop]);
 });
 });
});

Here's a jsfiddle

---

Of course, a much simpler approach would to be build elements as needed. Keep the CSS but remove all the HTML (except the indices div, just so we have a place to put stuff) in favor of letting your JS do the work of setting it all up.

var symbols = ['^GSPC', '^NDX', 'GLD', 'SLV', '^N225', '^FTSE'],
 properties = [
 { classname: 'name', property: 'Name' },
 { classname: 'result', property: 'LastTradePriceOnly' },
 { classname: 'change', property: 'Change' }
 ];
function buildElement(quote) {
 var container = $("<div></div>").addClass("symbol");
 properties.forEach(function (prop) {
 var child = $("<span></span>").addClass(prop.classname);
 child.text(quote[prop.property]);
 container.append(child);
 });
 if(/^\+/.test(quote.PercentChange)) {
 container.addClass("up");
 } else {
 container.addClass("down");
 }
 return container;
}
$.getJSON("http://query.yahooapis.com/v1/public/yql", {
 format: "json",
 diagnostics: "true",
 env: "http://datatables.org/alltables.env",
 q: "select * from yahoo.finance.quotes where symbol in ('" + symbols.join("','") + "')"
}, function (data, xhr, status) {
 // do some sanity checking of the data here
 var elements = data.query.results.quote.map(buildElement);
 $("#indices").append(elements);
});

Here's a jsfiddle of that.

• Frank Tudor: Thank you so much. This is absolutely excellent feedback. Not only did you explain my problems but provided 'two' possible solutions for refactoring my code!