Implementing Strategy Design Pattern Using Buttons


#1

I have two buttons on my page and a div that is initially empty. Once a button is clicked the div is populated with an unordered list. I'm trying to practice implementing the Strategy pattern but I'm getting errors saying "missing a curly brace before the body". I know that the error message is incorrect and misleading because I validated the script online and it is syntactically valid. Below are my markup and script.

var List = function (_list, identifier) {
    'use strict';
    this.list = _list;
    this.identifier = identifier;
}

List.prototype.GetList = function () {
    return this.list();
}

var myList1 = function (identifier){
    // create List1   
    return "<ul id='" + identifier + "'><li>Test1</li><li>Test1</li></ul>";
}

var myList2 = function (identifier) {
    // create List2
    return "<ul class='" + identifier + "'><li>Test2</li><li>Test2</li></ul>";
}

function CreateList (myList, identifier) {
    var newlist = new List (myList, identifier);
    var div = document.getElementById ("DisplayArea");
    div.innerHTML = "";
    div.innerHTML = newlist.GetList (identifier);
}

<button onclick="CreateList(myList1, 'List1')"> Create List1
</button>
<button onclick="CreateList(myList2, 'class1')"> Create List2
</button>

<div id = "DisplayArea">
<div>

#2

Implementation of design patterns is best held off until you actually have a requirement to change algorithms on the fly.

When you have that type of problem, there are steps that are performed to get you to the strategy design pattern.

Here are the steps that you perform to achieve a strategy design pattern.

  1. Identify an algorithm that the client would prefer to access through a "flex point."
  2. Declare the common interface for all variations of the algorithm.
  3. One-by-one, extract all algorithms into their own classes. They all should follow the common Strategy interface.
  4. Add a field for storing a reference to the current strategy, as well as a setter to change it, into the Context class. The Context should work with this object only using the Strategy interface.
  5. Context's clients must provide it with a proper strategy objects when they need the Context to perform the work in a certain way.

For further details, please see https://refactoring.guru/design-patterns/strategy


#3

The criticism page on design patterns specifically mentions the strategy pattern saying:

i.e., don't do it.


#4

Hi Paul, thanks for your reply. I am familiar with using the Strategy pattern in C# but I haven't used it in Javascript as it doesn't have the concept of an interface.

I'm just exploring how Javascript implements various design patterns without using interfaces. I'm trying to figure out why the compiler is complaining.


#5

What is it complaining about?


#6

If I comment out myList2, the first button works just fine. The compiler is giving me the errors

SyntaxError: missing { before class body
ReferenceError: CreateList is not defined

Below is my page markup

<!doctype html>
<head>
<meta charset="UTF-8">
<title> Testing Strategy Pattern </title>
<style>

   #Buttons{width:100%; height:50px; text-align:center}
   buttons{width:10%; height:50px; text-align:center}
   #DisplayArea{
     width:100%; height:200px;    
   }

   ul{ width:100%; height:100%; margin:0 auto; 
       padding:0; text-align:center; list-style:none;
   }

   ul li{ width:25%; height:20%; 
          margin:2%;
          font-size:24px;
          color:white; background:black;
          display:inline-block;
          line-height:35px;
   }

</style>
<script>

var List = function (_list, identifier) {
    'use strict';
    this.list = _list;
    this.identifier = identifier;
}

List.prototype.GetList = function () {
    return this.list();
}

var myList1 = function (identifier){
    // create List1   
    return "<ul id='" + identifier + "'><li>Test1</li><li>Test1</li></ul>";
}

var myList2 = function (identifier) {
    // create List2
    return "<ul class='" + identifier + "'><li>Test2</li><li>Test2</li></ul>";
}

function CreateList (myList, identifier) {
    var newlist = new List (myList, identifier);
    var div = document.getElementById ("DisplayArea");
    div.innerHTML = "";
    div.innerHTML = newlist.GetList (identifier);
}
</script>
</head>
<body>
<div id="Buttons">
<button onclick="CreateList(myList1, 'List1')"> Create List1
</button>
<button onclick="CreateList(myList2, 'class1')"> Create List2
</button>
</div>
<div id = "DisplayArea">
<div>
</body>
</html>

#7

The code works well for me when I test it in a web browser with either button.


#8

You mentioned a compiler. Which compiler are you using?


#9

I'm using firefox. I haven't tested it in other browsers.


#10

Good one, I'm using Firefox too for testing your code. You may be used to speaking about compiling from C# but JavaScript is not compiled - it's an interpreted language instead.

When testing your code from post #6 in Firefox, I don't seem to get any errors, either on the page in the browser console.


#11

Wow that is strange. I've tested in Chrome, Firefox, MS Edge, and IE and now everything just works.


#12

Here are some proposed improvements to your code.

Replace inline event listeners with standard event listeners

The event listeners should never be assigned from the HTML code. There are several reasons for this, but two of the main ones that come to mind are that you want people to be able to edit the HTML code without putting the scripting at risk, and it's easier to manage large numbers of items when it's not directly tied into the HTML code.

<div id="Buttons">
    <button>Create List1</button>
    <button>Create List2</button>
</div>

The below script I'll put into a separate script tag at the end of the body, just before the </body> tag. This is a good practice as it allows the script to most quickly and easily work with DOM elements on the page.

var buttons = document.querySelectorAll("#Buttons button");
buttons[0].addEventListener("click", function () {
    CreateList(myList1, 'List1');
});
buttons[1].addEventListener("click", function () {
    CreateList(myList2, 'class1');
});

Simplify the event handlers

The HTML code became simpler, and now the scripting code needs to be made simpler too. This can be easily achieved by putting the arguments into an array.

var buttonList = [
    {name: "List1", callback: myList1},
    {name: "class1", callback: myList2}
];
var buttons = document.querySelectorAll("#Buttons button");
buttons.forEach(function (button, index) {
    var list = buttonList[index];
    button.addEventListener("click", buttonClickHandlerfunction () {
        CreateList(list.callback, list.name);
    });
});

Move the scripting code to the end of the body

The other parts of the script that are currently in the head I'll move down to the end of the body too.

<script>
// code moved down below
</script>
</head>
<body>
...
<script>
// code moved into here
...
</script>
</body>

The scripting code is now all together:

var List = function (_list, identifier) {
...
    div.innerHTML = newlist.GetList (identifier);
}

var buttonList = [
...
    });
});

Use correct naming conventions

Functions with a capital first letter are normally reserved for constructor functions, so this is a good time to rename the function to createList instead.

// function CreateList (myList, identifier) {
function createList (myList, identifier) {
...
        // CreateList(list.callback, list.name);
        createList(list.callback, list.name);

The List function and the other functions should also be declared as function declarations instead of function expressions, as the anonymous function expression version tends to result in more confusion and no significant benefits otherwise.

// var List = function (_list, identifier) {
function List(_list, identifier) {

Proper use of "use strict"

The "use strict" currently only applies to the List function. Move it out to a containing function to gain the full benefits of using "use strict". In this case, I'll use an IIFE (an Immediately Invoked Function Expression) which is a good practice to use, as it helps to protect the global namespace from your code, and it's easier to later on modularize code.

(function iife() {
    "use strict";
    // all scripting code in here
}());

Removing underscore prefix

Using the underscore prefix is a bad practice that's brought over from other programming languages. JavaScript knows what you mean without the prefix.

// function List(_list, identifier) {
function List(list, identifier) {
    // this.list = _list;
    this.list = list;
    this.identifier = identifier;
}

Directly invoke the function

Looking at the the createList() function where the List() function is used, big gains can be achieved here.

    function createList(myList, identifier) {
        var newlist = new List (myList, identifier);
        var div = document.getElementById ("DisplayArea");
        div.innerHTML = "";
        div.innerHTML = newlist.GetList(identifier);
    }

The GetList function just invokes the myList variable, so we can replace all of that code instead with just an invocation of the myList variable.

    function createList(myList, identifier) {
        var div = document.getElementById ("DisplayArea");
        div.innerHTML = myList(identifier);
    }

This has helped to demonstrate that in your current usage, there's absolutely no need for the strategy design pattern.

All of the List() function code can now be removed, and the code still works fine.

    // function List(list, identifier) {
    //     this.list = list;
    //     this.identifier = identifier;
    // }

    // List.prototype.GetList = function () {
    //     return this.list();
    // }

Use function declarations for functions

The other myList1 and myList2 functions can be defined as function declarations too, to help remove any questions about why they were defined as anonymous functions instead.

    // var myList1 = function (identifier){
    function myList1(identifier){
        return "<ul id='" + identifier + "'><li>Test1</li><li>Test1</li></ul>";
    }

    // var myList2 = function (identifier){
    function myList2(identifier) {
        return "<ul class='" + identifier + "'><li>Test2</li><li>Test2</li></ul>";
    }

Place configurable data on the HTML elements themself

Can we entirely remove that buttonList array that's currently managing things? What if we had the data on the elements themself?

<div id="Buttons">
    <button data-id="List1">Create List1</button>
    <button data-class="class1">Create List2</button>
</div>

The scripting code can now check that those data attributes exist, and call the appropriate function:

    // var buttonList = [
    //     {name: "List1", callback: myList1},
    //     {name: "class1", callback: myList2}
    // ];
    var buttons = document.querySelectorAll("#Buttons button");
    // buttons.forEach(function (button, index) {
    buttons.forEach(function (button) {
        // var list = buttonList[index];
        // button.addEventListener("click", function () {
        button.addEventListener("click", function (evt) {
            // createList(list.callback, list.name);
            var el = evt.target;
            if (el.dataset.id) {
                createList(myList1, el.dataset.id);
            } else if (el.dataset.class) {
                createList(myList2, el.dataset.class);
            }
        });
    });

The role that those myList1() and myList2() functions play now becomes much more apparent, to the point where you might want to rename them to something more specific, such as showIdList() and showClassList() instead, but the code is much improved now over what it was before.

Use a separate event handler function

We can now also move the eventHandler code out to a separate function, which helps to simplify the event handling process.

    function buttonClickHandler(evt) {
        ...
    }
    var buttons = document.querySelectorAll("#Buttons button");
    buttons.forEach(function (button) {
        /* button.addEventListener("click", function (evt) {
            ...
        }); */
        button.addEventListener("click", buttonClickHandler);
    });

After the improvements

After that HTML code improvement, the JavaScript code is now much simplified, with the full scripting code being:

(function iife() {
    "use strict";
    function myList1(identifier) {
        return "<ul id='" + identifier + "'><li>Test1</li><li>Test1</li></ul>";
    }

    function myList2(identifier) {
        return "<ul class='" + identifier + "'><li>Test2</li><li>Test2</li></ul>";
    }

    function createList(myList, identifier) {
        var div = document.getElementById("DisplayArea");
        div.innerHTML = myList(identifier);
    }

    function buttonClickHandler(evt) {
        var el = evt.target;
        if (el.dataset.id) {
            createList(myList1, el.dataset.id);
        } else if (el.dataset.class) {
            createList(myList2, el.dataset.class);
        }
    }

    var buttons = document.querySelectorAll("#Buttons button");
    buttons.forEach(function (button) {
        button.addEventListener("click", buttonClickHandler);
    });
}());

I put together a jsfiddle example showing this simplified and easier to understand code at https://jsfiddle.net/83yuy0e5/4/


#13

Hi Paul, Thanks for taking the time to explain things.


#14

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.