SitePoint Sponsor

User Tag List

Results 1 to 25 of 33

Threaded View

  1. #22
    Unobtrusively zen silver trophybronze trophy
    paul_wilkins's Avatar
    Join Date
    Jan 2007
    Location
    Christchurch, New Zealand
    Posts
    14,729
    Mentioned
    104 Post(s)
    Tagged
    4 Thread(s)
    Quote Originally Posted by paul_wilkins View Post
    Tomorrow we should be able to use a similar selector for the container, and move the code below the roll function in to its own separate function for easy handling.
    That should help us to easily call the lotto function with our own custom selectors for the container and template, instead of needing to have them embedded within the lotto code.
    We can now fix up the container side of things, which currently starts out as:

    Code:
    container = document.getElementById('container'),
    templateSelector = '.lotto.template',
    playSelector = 'input',
    resultSelector = 'div';
    ...
    var content = document.querySelector(templateSelector).cloneNode(true),
    ...
    container.innerHTML = '';
    container.appendChild(content);
    We can use the same selector technique for the container. While we are here, I'll also make the play and result selectors a bit more specific too

    HTML Code:
    <input type="button" class="play">
    <div class="result"></div>
    Code javascript:
    containerSelector = '.lotto.container',
    templateSelector = '.lotto.template',
    playSelector = '.play',
    resultSelector = '.result';
    ...
    var container = document.querySelector(containerSelector),
        content = ...

    What we want to do is to move these selectors in to the opts object, but before we do that, we should first refactor the below code in to separate functions.

    Code:
    var container = document.querySelector(containerSelector),
        content = document.querySelector(templateSelector).cloneNode(true),
        playButton = content.querySelector(playSelector),
        resultDiv = content.querySelector(resultSelector);
    
    removeClass(content, 'template');
    
    playButton.id = "play" + idx;
    playButton.value = opts.buttonText;
    playButton.onclick = function justOnce() {
        this.blur();
        if (counter > 0) {
            return false;
        }
        roll();
    };
    
    resultDiv.id = "result" + idx;
    resultDiv.innerHTML = opts.initialText;
    
    container.innerHTML = '';
    container.appendChild(content);
    What do we have there, is code to setup the content from that template, and to update the container with that content.
    That's a lot of code for just one function, due to several responsibilities being involved, so what I want to do instead is to call an init function to get things going. That init function can then pass the opts info to where it's needed.

    Code javascript:
    init(opts) {
        var container = document.querySelector(opts.containerSelector),
            template = document.querySelector(opts.containerSelector),
            content = generateContentFromTemplate(template, opts);
     
        updateContainer(container, content);
    }

    That's how I want things to be structured.

    With the generateContentFromTemplate() function we can currently move the existing code in to that function.

    Code javascript:
    function generateContentFromTemplate(template, opts) {
        var content = document.querySelector(selector).cloneNode(true),
        playButton = content.querySelector(playSelector),
        resultDiv = content.querySelector(resultSelector);
     
        removeClass(content, 'template');
     
        playButton.id = "play" + idx;
        playButton.value = opts.buttonText;
        playButton.onclick = function justOnce() {
            this.blur();
            if (counter > 0) {
                return false;
            }
            roll();
        };
     
        resultDiv.id = "result" + idx;
        resultDiv.innerHTML = opts.initialText;
     
        return content;
    }
    ...
    content = generateContentFromTemplate(templateSelector, opts);

    We will come back to this later on, after the restructuring has occurred.

    After moving that code in to the function, the roll function can't find the resultDiv variable. Because two different places want to update the resultDiv, this is a good time to create a function called updateResult just for that purpose, where we can use the selector in opts.resultSelector to specify where the update occurs.

    Because the result that we want to update may be on the page, or it may be in the template content that we are working with, we will want to pass a context to the updateResult function too. We can also default that to the document object if none is provided.

    Code javascript:
    function updateResult(html, context) {
        context = context || document;
     
        var resultSelector = opts.resultSelector,
            result = context.querySelector(resultSelector);
     
        result.innerHTML = html;
    }
    ...
    var draw = ...,
        container = document.querySelector(containerSelector);
     
    ...
    updateResult(draw.join(' '), container);

    and the following line from generateContentFromTemplate() can be replaced too:

    Code:
    resultDiv = content.querySelector(opts.resultSelector);
    ...
    resultDiv.innerHTML = opts.initialText;
    updateResult(opts.initialText, content);
    Things are now ready for us to move the selectors in to the opts object.
    We can also get the selector first from the options passed to the lotto function, before defaulting to our default selectors. That way they can be overridden when calling the lotto function.
    We do though need to protect ourself from when lotto is called with no options.

    Code javascript:
    if (!options) {
        options = {};
    }
    var opts = {
        pick: options.pick || 6,
        from: options.from || 1,
        to: options.to || 49,
        containerSelector: options.containerSelector || '.lotto.container',
        templateSelector: options.templateSelector || '.lotto.template',
        playSelector: options.playSelector || '.play',
        resultSelector: options.resultSelector || '.result',
        totalRolls: options.totalRolls || 50,
        rollDelay: options.rollDelay || 50,
        buttonText: options.buttonText || "Lotto Lucky Dip",
        initialText: options.initialText || "Your Lucky Numbers"
    },

    They are listed in an approximate order of importance, but it's entirely up to you. You can list them, for example, in alphabetical order instead of yolu like.

    The move to the opts function means that some pieces of code now need to be updated, to retrieve the information from there.
    Thanks to the restructuring though, it's not many pieces at all though.

    Code javascript:
    var result = context.querySelector(opts.resultSelector);
    ...
    container = document.querySelector(opts.containerSelector);
    ...
    playButton = content.querySelector(opts.playSelector),
    resultDiv = content.querySelector(opts.resultSelector);
    ...
    var container = document.querySelector(opts.containerSelector),
    template = document.querySelector(opts.templateSelector);

    Another change that I'll want to make is to have the defaults specified separately, and to then loop through them adding them to any missing properties from the options, but that's for later on the todo list.

    Let us now get back to that generateContentFromTemplate function. Here's the current code that we'll be working on.

    Code:
    function generateContentFromTemplate(template, opts) {
        var content = template.cloneNode(true),
            playButton = content.querySelector(opts.playSelector),
            resultDiv = content.querySelector(opts.resultSelector);
    
        removeClass(content, 'template');
        
        playButton.id = "play" + idx;
        playButton.value = opts.buttonText;
        playButton.onclick = function justOnce() {
            this.blur();
            if (counter > 0) {
                return false;
            }
            roll();
        };
    
        resultDiv.id = "result" + idx;
        updateResult(opts.initialText, content);
    
        return content;
    }
    Here's what needs to be done with it:
    • the idx parts aren't needed at all
    • the play section could do with being extracted out to its own function
    • I'm not happy with that removeClass section


    Removing the idx parts is easy, and the variable declaration further up for is can be removed too.

    Code:
    idx = document.getElementsByTagName('div').length,
    ...
    playButton.id = "play" + idx;
    resultDiv.id = "result" + idx;
    The playButton code can be moved in to its own function, and called from within the generateContentFromTemplate one:

    Code javascript:
    function setupPlayButton(button, value) {
        button.value = value;
        button.onclick = function rollLottoNumbers() {
            this.blur();
            if (counter > 0) {
                return false;
            }
            roll();
        };
    }
    ...
    setupPlayButton(playButton, opts.buttonText);

    With the removeClass part, not only does it seem like a needless detail but it also results in the following nested div HTML structure for the container:

    HTML Code:
    <div class="lotto container">
        <div class="lotto">
            <!-- template content in here -->
        </div>
    </div>
    What would be good instead, is to remove that inner div - to unwrap it from the content.

    Let's move the problem out to a separate function, so that we can clean up the generateContentFromTemplate() function and focus on this last remaining problem.

    Code javascript:
    function generateContentFromTemplate(template, opts) {
        var content = getTemplateContent(template),
            playButton = content.querySelector(opts.playSelector),
            resultDiv = content.querySelector(opts.resultSelector);
     
        setupPlayButton(playButton, opts.buttonText);
        updateResult(opts.initialText, content);
     
        return content;
    }

    That function is now looking a lot better than it was.

    The getTemplateContent() function starts off like this:

    Code:
    function getTemplateContent(template) {
        var content = template.cloneNode(true);
        removeClass(content, 'template');
        return content;
    }
    What I want to do here is to create a document fragment, and copy the child nodes of the template over to there instead.

    Code javascript:
    function getTemplateContent(template) {
        var copyOfTemplate = template.cloneNode(true),
            content = document.createDocumentFragment();
     
        while (copyOfTemplate.hasChildNodes()) {
            content.appendChild(copyOfTemplate.firstChild);
        }
     
        return content;
    }

    And now the class-handling functions, such as removeClass that we had earlier, can now be removed.

    Lastly for now, the form-handling code should be moved out of the wrapper for the lotto code, and placed at the end below the wrapper. This allows the lotto code to be easily moved out to a separate javascript file.

    So now the lotto code can be started with just a call to lotto(), or other options can be given to it to change how it behaves.

    JSLint is still happy with all of the code, when we tell it to assume that we're using a browser, and there isn't much more with the refactoring left to be done.
    The code that we now have is found at http://jsfiddle.net/pmw57/8R42e/22/

    I hope you can agree that this refactoring has been worth it.

    Off Topic:

    Mad props to the SitePoint developers.

    I foolishly performed a web browser update while the preview for this post was left in an open tab, and I was able to retrieve ALL of this super-long post using the auto-restore content link.

    It seriously helps to alleviate a bad situation for me.
    Last edited by paul_wilkins; May 4, 2013 at 14:00. Reason: adjust a double "we can now" situation
    Programming Group Advisor
    Reference: JavaScript, Quirksmode Validate: HTML Validation, JSLint
    Car is to Carpet as Java is to JavaScript


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •