3
\$\begingroup\$

My goal is to use the object literal pattern in TypeScript.

Here is an example of the object literal pattern in JavaScript.

I created a simple example of the object literal pattern in TypeScript. The only change I made compared to plain JavaScript was to declare an interface for the object literal.

See the full working example: Codepen

I'm curious to know if I'm missing out on any TypeScript features or not following any best practices.

interface square {
    squares: NodeListOf<Element>;
    squareColors: Array<string>;
    squareSizer: (el: HTMLElement) => void;
    squareColorChanger: (el: HTMLElement) => void;
    doSquareClick: (element: HTMLElement, index: number, array: Array<HTMLElement>) => void;
    init: () => void;
}

let squareWidget: square = {

    squares: document.querySelectorAll(".square"),

    squareColors: ["square-is-color-red", "square-is-color-orange", "square-is-color-green"],

    squareSizer: function (el) {
        if (el.dataset.enlarged === "false") {
            el.classList.add("square-is-enlarged");
            el.dataset.enlarged = "true";
        } else if (el.dataset.enlarged === "true") {
            el.classList.remove("square-is-enlarged");
            el.dataset.enlarged = "false";
        }
    },

    squareColorChanger: function (el) {
        for (var i = 0; i < el.classList.length; i++) {
            if (el.classList[i].indexOf("square-is-color") !== -1) {
                el.classList.remove(el.classList[i]);
            }
        }

        var randomColorIndex = Math.floor(Math.random() * (squareWidget.squareColors.length));
        el.classList.add(squareWidget.squareColors[randomColorIndex]);
    },

    doSquareClick: function (element, index, array) {
        element.addEventListener("click", function () {
            squareWidget.squareSizer(element);
            squareWidget.squareColorChanger(element);
        });
    },

    init: function () {
       Array.from(squareWidget.squares).forEach(squareWidget.doSquareClick);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Why not use a regular TypeScript class? It is more natural in this language. \$\endgroup\$ – Igor Soloydenko May 12 '17 at 0:31
  • \$\begingroup\$ @IgorSoloydenko I strongly disagree. class is an ECMAScript feature and TypeScript's type system is actually as attuned to functional language constructs as OO ones if not more so. \$\endgroup\$ – Aluan Haddad May 12 '17 at 4:50
  • 1
    \$\begingroup\$ @AluanHaddad 1. TypeScript is superset of ECMAScript. ;) 2. The author of the question defines a bunch of things and refers to this set via a single name let squareWidget: square = { listOfFunctions }. How is it different from a [static] class with static methods? How does my proposal make the code less functional (this is a case where OOP does not replace FP, but adds to it)? And again, why use the object-literal thing if there are classes which are more natural for most developers when it comes to OOP especially when language provide classes as a concept? \$\endgroup\$ – Igor Soloydenko May 12 '17 at 5:45
  • \$\begingroup\$ I know it is a superset, my point is that class does not come from TypeScript and, more importantly, that OOP and TypeScript are orthogonal. Also, a class with static methods is more complex than an object literal. Classes with only static members in TS/JS are pointless complexity. I agree that a class is not necessarily less functional. Ironically, I prefer factories as they allow for proper encapsulation. Seeing class as the natural way to write TypeScript or JavaScript suggests an over exposure to Angular. Angular is a poor example of TypeScript. \$\endgroup\$ – Aluan Haddad May 12 '17 at 5:51
  • \$\begingroup\$ @AluanHaddad does not come from TypeScript so what? class is available in TypeScript anyway... My point is that OOP and TypeScript are orthogonal. -- Totally agree. Also, a class with static methods is more complex than an object literal. -- more complex in which way, and when compared against which definition of complexity? \$\endgroup\$ – Igor Soloydenko May 12 '17 at 5:54
1
\$\begingroup\$

I'm not going to suggest any major Typescript features you aren't using, as it looks like you don't need any here. Additionally, I'm not going to claim any of my following questions / suggestions are "best practices" as "best practices" seem to change annually at a minimum in the web development world.


Why are you defining an interface for a single object? I don't see any reason to burden yourself with maintaining multiple definitions. You can include the method definitions within the module itself to save yourself from scrolling back up to the interface to check what the definition should be.

This is more of a stylistic preference, but I see no need to preface your methods with square. Since you are accessing the method on the squareWidget object it seems redundant to include square a second time.

When squareWidget is created, the list of squares is immediately created. This is a problem as if you include your code before the HTML has finished loading, the squares added before calling squareWidget.init(), but after the code is parsed will be missed.

In your squareSizer function, you use a dataset value which is not set by the init function. If the value is missing, the method won't do anything. When creating a square, should I have to remember to specify if it is enlarged or not every time? Also, is the dataset used in any code not included in your post? If not, you could replace your checks with the built in classList.toggle()

In squareColorChanger, instead of looping through the classes on the element, it might be better to check for and remove the color classes specified by the module. You can even take this a step further and just use el.classList.remove(...squareWidget.squareColors)

In doSquareClick I expected to see code that handled a click. Instead, the method adds an event listener to the element. Perhaps addListener would be more descriptive? In addition to this, your method asks for more arguments than it uses. You can safely remove the index and array arguments with no effect to your code.

In init the Array.from is unnecessary in newer browsers. forEach is included on NodeLists. Despite this, I would leave it as is until IE dies out completely. One concern I have with the current function is that if I call it twice, two event listeners will be added on all elements. If I add squares to the page after it has loaded due to user interaction I need to call the init method again to activate them which then breaks the existing squares. You could solve this problem several ways. My personal choice would likely be defining a single event listener method which is added on all squares. With this change, calling addEventListener again will have no effect.

| improve this answer | |
\$\endgroup\$

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.