HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Data item prototype in a Javascript data structure where object instance functions see own data

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
itemjavascriptwhereseeinstanceownprototypestructurefunctionsobject

Problem

I'm working on a complex Javascript application which includes its own data structure. The previous version's data structure was rather chaotic, and I'm looking to replace it with something where:

  • Every data item is an object which is (effectively) an instance of a class, which imposes a common structure including default values



  • Where possible, functions which set or get the data in a data item are taken out of the controller logic and attached to the data item class prototype



The problem I'm trying to solve by introducing this structure is, lots of messy and inconsistent 'model' style logic buried deep in 'controller' style functions, which has led to inconsistency, type errors, missing data errors, duplicated logic, etc.

Here's an example which uses the exact same approach I'm taking to defining and accessing instances (but the contents of data structure itself is simplified, since my question is about the way it's defined and accessed). I know that Javascript doesn't exactly have object 'classes' the way other languages do, and I've read about approximating them using prototypes, but I may be doing it wrong. In particular:

  • Do I need an init() function? Seems fine without one.



  • Is this a robust approach to the instance object accessing its own data? Can I rely on this within the attached function always being the instance's own data each time the function is called? Or should I be prepared to use call() or wrap calls in an anonymous function, or set it as a variable like that or self?



  • Is this a good way to give a class of objects default values? Note that this application already uses jQuery for other non-negotiable reasons, hence $.extend, and namespacing is already covered



  • I put the defaults object in the prototype so it's not redefined each time a new instance is created. Am I right in thinking this is a better practice than defining it within the function?



  • My actual application isn't about currencies, so please don't comment

Solution

Naming Things

The class name dataItem doesn't communicate what it is, or how it is meant to be used:


There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors. - Martin Fowler

What is a dataItem? From the code, it is a number value and a currency symbol. Perhaps CurrencyFormat might be more appropriate because what you are doing is formatting a number as currency.

Furthermore, naming conventions in JavaScript require that constructor functions should begin with a capital letter (DataItem vs dataItem). This naming convention is reflected in the native JavaScript API as well (String, Number, XMLHttpRequest, ...).

Principle of Single Responsibility

Being that you could format multiple numbers as the same currency, you might want to limit the instances that are available and use them to format any number, rather than keeping track of the number as an instance variable. This is the principle of Single Responsibility (do one thing, and do it well).

You can then provide an instance for each supported currency type, and a factory function that returns the proper formatter based on the currency name (e.g. "USD"):

function CurrencyFormat(symbol, name) {
    this.symbol = currencySymbol;
    this.name = currencyName;
}

CurrencyFormat.prototype = {
    name: null,
    symbol: null,

    constructor: CurrencyFormat,

    format: function(n, decimalPlaces) {
        decimalPlaces = typeof decimalPlaces == "number" ? decimalPlaces : 2;

        return this.symbol + n.toFixed(decimalPlaces) + " " + this.name;
    }
}

CurrencyFormat.AUD = new CurrencyFormat("$", "AUD");
CurrencyFormat.CAD = new CurrencyFormat("$", "CAD");
CurrencyFormat.GBP = new CurrencyFormat("£", "GBP");
CurrencyFormat.USD = new CurrencyFormat("$", "USD");

CurrencyFormat.getFormatter = function(currencyName) {
    if (CurrencyFormat[currencyName] && CurrencyFormat[currencyName] instanceof CurrencyFormat) {
        return CurrencyFormat[currencyName];
    }

    throw new Error("Currency format not implemented: " + currencyName);
};


Given the values you posted, this is how you would use the class:

var taxes = 60;
var expenses = 199.5;
var canadaIncome = 0;

console.log(CurrencyFormat.GBP.format(expenses));
console.log(CurrencyFormat.CAD.format(canadaIncome));
console.log(CurrencyFormat.USD.format(taxes));


If you know the currency name, you can use CurrencyFormat.getFormatter(...) to get the proper object:

// Get a formatter object by name:
var formatter = CurrencyFormat.getFormatter("USD");

// Throws an error because the currency name is not implemented:
var formatter = CurrencyFormat.getFormatter("XYZ");


This allows you to inject a currency formatter into other objects:

function ShoppingCartItem(price, currencyFormatter) {
    this.price = price;
    this.currencyFormatter = currencyFormatter;
}

ShoppingCartItem.prototype.formatPrice = function() {
    return this.currencyFormatter.format(this.price);
}

var item1 = new ShoppingCartItem(105.95, CurrencyFormat.getFormatter("USD"));
var item2 = new ShoppingCartItem(105.95, CurrencyFormat.GBP);

console.log(item1.formatPrice());
console.log(item2.formatPrice());


Removing unnecessary dependencies

There is no need to use jQuery.extend for just a couple of properties. This is an unnecessary dependency. For something as simple as a currency symbol and name, just pass those values in as constructor arguments. Passing in an object of key-value pairs is like hitting a thumb tack with a sledge hammer in this case.

Even if you needed to set a dozen properties by passing in an object of key-value pairs, I still wouldn't use jQuery.extend. It's not magic. It's just a for-in loop. You are better off writing half a dozen lines of code in order to remove a dependency on a large JavaScript library when this one class uses a fraction of what jQuery offers.

Code Snippets

function CurrencyFormat(symbol, name) {
    this.symbol = currencySymbol;
    this.name = currencyName;
}

CurrencyFormat.prototype = {
    name: null,
    symbol: null,

    constructor: CurrencyFormat,

    format: function(n, decimalPlaces) {
        decimalPlaces = typeof decimalPlaces == "number" ? decimalPlaces : 2;

        return this.symbol + n.toFixed(decimalPlaces) + " " + this.name;
    }
}

CurrencyFormat.AUD = new CurrencyFormat("$", "AUD");
CurrencyFormat.CAD = new CurrencyFormat("$", "CAD");
CurrencyFormat.GBP = new CurrencyFormat("£", "GBP");
CurrencyFormat.USD = new CurrencyFormat("$", "USD");

CurrencyFormat.getFormatter = function(currencyName) {
    if (CurrencyFormat[currencyName] && CurrencyFormat[currencyName] instanceof CurrencyFormat) {
        return CurrencyFormat[currencyName];
    }

    throw new Error("Currency format not implemented: " + currencyName);
};
var taxes = 60;
var expenses = 199.5;
var canadaIncome = 0;

console.log(CurrencyFormat.GBP.format(expenses));
console.log(CurrencyFormat.CAD.format(canadaIncome));
console.log(CurrencyFormat.USD.format(taxes));
// Get a formatter object by name:
var formatter = CurrencyFormat.getFormatter("USD");

// Throws an error because the currency name is not implemented:
var formatter = CurrencyFormat.getFormatter("XYZ");
function ShoppingCartItem(price, currencyFormatter) {
    this.price = price;
    this.currencyFormatter = currencyFormatter;
}

ShoppingCartItem.prototype.formatPrice = function() {
    return this.currencyFormatter.format(this.price);
}

var item1 = new ShoppingCartItem(105.95, CurrencyFormat.getFormatter("USD"));
var item2 = new ShoppingCartItem(105.95, CurrencyFormat.GBP);

console.log(item1.formatPrice());
console.log(item2.formatPrice());

Context

StackExchange Code Review Q#88559, answer score: 11

Revisions (0)

No revisions yet.