My random quote machine spits out Donald Trump quotes; you can find it here. It uses the ‘what does trump think’ api.
When writing the JS (I didn’t use jQuery) for this I realised that I still don’t really know how to organise and structure my code. I tried to keep functions doing single tasks, but ended up with one which really does multiple tasks… Any useful criticism of the structure of my JS (or anything else) much appreciated! Pointers to straightforward resources on structuring code would be useful.
'use strict';
// global variable
var url;
// insert random quote on page load
getQuote();
// listens for new quote click and calls getQuote
document.getElementById('get-quote').addEventListener('click', function() {getQuote();});
// makes call to quote api, parses response, calls and passes quotation to showQuote and makeTweetLink
function getQuote() {
var xhr = new XMLHttpRequest();
xhr.open('GET', 'https://api.whatdoestrumpthink.com/api/v1/quotes/random');
xhr.send();
xhr.onload = function() {
if (xhr.status === 200) {
var response = JSON.parse(xhr.responseText);
showQuote(response.message);
makeTweetLink(response.message);
}
else {
showQuote('This is not a quotation, its an error message! Request status: ' + xhr.status)
}
}
}
// insert quotation into html
function showQuote(quote) {
document.getElementById('quote').innerHTML = quote;
}
// make tweet link (updates global variable 'url')
function makeTweetLink(quote) {
url = 'http://twitter.com/share?text=' + quote + ' (Donald Trump)';
}
// open tweet popup when tweet button clicked
document.getElementById('tweet').addEventListener('click', function() {
window.open(url, 'status=1', 'width=500, height=450');
})
EDIT: I updated my code as I found an error - didn’t want it to mislead anyone. Previously my getQuote() function included:
xhr.onload = function() {
if (xhr.readyState === xhr.DONE) {
if (xhr.status === 200) {```
I've removed the middle line:
`if (xhr.readyState === xhr.DONE) {`
I removed it because it is unnecessary. The onload event is only triggered when the readyState property === DONE (or 4, which is the same thing here); see [this stackoverflow answer](http://stackoverflow.com/questions/9181090/is-onload-equal-to-readystate-4-in-xmlhttprequest).
I was using the readyState === DONE because I had previously used the onreadystatechange as my trigger event, and for this event you do need to test the readyState of your request.
2 Likes
@Surogo No critique to offer. Nice idea. Funny and incredible at the same time.
This is the funniest project I’ve seen on here. Thank you for making me laugh.
As a side note, I think you should put the blue box at the center of the webpage. But if you did this to make the Trump background more visible I understand and would advise you put the box top/center-left or top/center-right.
The JavaScript looks fine to me. The idea is entertaining.
Bruce
AirBnB javascript style guide. Style guides suggest ways to structure html, css, and js files. You can group your event listeners together.
If your target audience is easily offended by cuss words, you may need to add some functionality to cover up f**k in one of the quotes.
@Reggie01 Thanks, I guess a style guide is probably what I was looking for; also good point about the cuss words, hope you weren’t offended 
Thanks for the positive feedback everyone!
I’ve never seen the words “Trump” and “think” used in one sentence before.
I was under the impression that words just spill out from his mouth randomly.
1 Like
What if you make the blue box match the blue box this color (#006da3 / rgb(0, 109, 163)) and make a little translucent.
Then stretch it out so it’s wider but not as tall and center it or at least move it up so it’s not stuck to the bottom. Currently it sits right above my taskbar. It goes right to the bottom of the screen when the bar hides.
I think your code looks good. The function that does more than one thing has to be that way because the xml request is asynchronous, and you must use the resulting quote inside the fucntion (ie get the tweet, render the quote).
I’d suggest to imporve & expand your JS knowledge, look into using fetch() and look into using promises as an alternative to the xml request & asynch call backs.
Good work.
1 Like
@the-thief I agree its not great having it stuck right at the bottom; my plan was to make it cover the lectern as I felt it fit well there. The trouble with that is that as the page resizes the box no longer covers the lectern. I’ll have to give this some more thought…
I chose the colours because they should match the US flag (according to Wikipedia) which is in the photo background.
Pretty funny 
I’m still waiting for the random protest machine… 
A link to the original source under the quote would be great!
Thanks Marc, thats a nice idea. Unfortunately the api does not include sources (although the ultimate source should be apparent!).
I’m having the same issue with the Twitter intent link showing up inside the Twitter window. Any idea of why that is happening? Struggling to debug it.
Thanks in advance!
This is a two year old topic: if you have an issue with your code, please open up a new topic, and share a link to your codepen or wherever else you’ve hosted your code. Thanks.
1 Like
Doing it in a sec. Thanks
