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.
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&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.
-
\$\begingroup\$ "intermingling JavaScript and jQuery". Well, anything else would be impossible. jQuery is written in JavaScript. JavaScript is the language; jQuery is just another library. \$\endgroup\$Flambino– Flambino2014年05月19日 21:41:04 +00:00Commented May 19, 2014 at 21:41
1 Answer 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. - 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 likediv.green
is what you need, in which case the class name would just begreen
. 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. - 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.
- 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&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]);
});
});
});
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.
-
\$\begingroup\$ 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! \$\endgroup\$Frank Tudor– Frank Tudor2014年05月20日 00:08:03 +00:00Commented May 20, 2014 at 0:08