0
\$\begingroup\$

I have the following code which loads in a URL based on the id of a user-clicked button. I'm looking to compress it down as much as possible, and make it more modular and reusable:

<!-- Function to load in info from different docs -->
function loadDoc(UrlString) {
  var xhttp = new XMLHttpRequest();
  xhttp.onreadystatechange = function() {
    if (xhttp.readyState == 4 && xhttp.status == 200) {
      document.getElementById("storybox").innerHTML = xhttp.responseText;
    }
  };
  console.log("URL = " + UrlString);
  xhttp.open("GET", UrlString, true);
  xhttp.send();
}

function buttonClicked(buttonid)  {
  console.log("entered case statement");
  var UrlString = "templates/story/pt1/Start.html";
  switch(buttonid) {
    case "YesStart":
        UrlString = "templates/story/pt1/YesStart.html";
        console.log("test");
        loadDoc(UrlString);
        break;
    case "Sleep":
       UrlString = "templates/story/pt1/Sleep.html";
        loadDoc(UrlString);
        break;
     case "Swindon":
        UrlString = "templates/story/pt1/Swindon.html";
        loadDoc(UrlString);
        break;
    default:
        document.getElementById("storybox").innerHTML = "<p>That part of the story hasn't been written yet. Click to</p><button type='button' class='btn btn-danger' id='nobtn' onclick='buttonClicked('YesStart')'>Restart</button>";
        <!-- Output error info -->
        console.log(buttonid);
    };
};

The case statement actually goes on a lot longer but I've cut it down. What I want to do is to shorten this code down as much as possible, so it dynamically creates URLs based on buttonid. Does anyone have an ideas about the best way to do this?

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CR! Please edit your title to tell us what your code is doing, instead of what you'd like us to do with it; that way we can prevent our site from becoming flooded with "How can I improve this code?" type of question titles. Thanks! \$\endgroup\$ – Mathieu Guindon May 4 '16 at 15:47
1
\$\begingroup\$
function loadDoc(UrlString) {
  var xhttp = new XMLHttpRequest();

First, I'd suggest replacing that XHR code with the newer fetch. There's a polyfill for it for older browsers. fetch returns Promises, which should be analogous to monads if I'm not mistaken.

xhttp.onreadystatechange = function() {
  if (xhttp.readyState == 4 && xhttp.status == 200) {
    document.getElementById("storybox").innerHTML = xhttp.responseText;
  }
};

Next would be to pull in the functionality that updates HTML to the caller instead of pushing it out to your AJAX operation. One big problem with your approach is that you're pushing out functionality all over the place. This becomes maintenance headache as the logic is all over the place. Should I want to change storybox to something else, I would have to trace all the way to loadDoc.

For that big switch statement, use an object as a map instead. Have it hold the url values. When a key doesn't exist, it should return undefined which you can easily default using || and a right-hand-side value.

In the end, your code can be as simple as:

var defaultUrl = 'templates/story/pt1/Start.html';

var urls = {
  YesStart: 'templates/story/pt1/YesStart.html',
  Sleep: 'templates/story/pt1/Sleep.html',
  Swindon: 'templates/story/pt1/Swindon.html'
};

function buttonClicked(buttonid){
  fetch(urls[buttonid] || defaultUrl).then(function(response) {
    return response.text()
  }).then(function(body) {
    document.getElementById("storybox").innerHTML = body
  })
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you! This is absolutely loads better than my code was! I think I might even shorten the code even further, using a single variable for the directory names, then concatenating with the filenames... although, would this have an impact on performance at all? \$\endgroup\$ – What's in a Google Search May 4 '16 at 16:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.