Encapsulating game data

Question

Consider such a code (javascript):

var jsonTmp = '{ "groups": [ { "name": "G1", "status": "inactive" }, { "name": "G2", "status": "active"}, { "name": "G3", "status": "inactive" }] }'
var dataTmp = JSON.parse(jsonTmp)

function g(key) {
    this.key = key;
    this.name;
    this.status; 

    this.setStatus = function (status) {
        this.status = status;
    }

    this.setName = function (name) {
        this.name = name;
    }

}

function main() {
    this.groups = []

    this.createGroups = function () {
        $.each(dataTmp.groups, function (key, group) {
             myGame.groups[key] = new g(key)
             myGame.groups[key].setStatus(group.status)
             myGame.groups[key].setName(group.name)
        })
    }

    this.getGroupsWithStatus = function (status) {
        var groups = [];
        $.each(myGroups, function (key, group) {
            if ( group.status == status ) { groups.push( key )}
        })
        return groups
    }


}

var myGame = new main();
var myGroups = myGame.groups
myGame.createGroups();
myGame.getGroupsWithStatus("inactive")

I am attempting to make good and effective, also readable OOP logic, and since it is my first fully oop project please make some advices:

I create game (myGame), that takes dynamically (from jsonTmp) data to create multiple objects, called groups. Because amount of groups may be different they do not have a name like

var group1 = new g("abc") var group2 = new g("cde")

They are stored without a instialized name in array of objects (here myGame.groups or myGroups).

Question 1: is that a good idea of storing objects to access them via key of object in array? I will be also searching for particular value of property of objects - like status here with getGroupsWithStatus. Do you have any better ideas to solve such a logic?

Question 2: can i set a name of instance in different way then

var instanceName = new g("abc")

and do i really need it if my objects are stored in array?

Question 3: In createGroups method I would rather use some kind of pointer instead of myGame.groups[key]:

  this.createGroups = function () {
        $.each(dataTmp.groups, function (key, group) {
        var currentGroup;                 
        currentGroup = new g(key)
        currentGroup.setStatus(group.status)
        currentGroup.setName(group.name)
        myGame.groups[key] = currentGroup;
        })
    }

I guess that solution is more readable. Am i right?

Sorry for maybe dumb questions, but i always used structural languages and oop logic is quite hard for me to use, and most of tutorials say about how to make class or inherit props of objects. Maybe you can also provide some good oop in practice tutorials? Thansk in advance. Kalreg.

Answer 1

• Q1: Sure
• Q2: Technically you are not setting a name, and that variable is hence misleading
• Q3: You could do that, but I would propose you just send the whole object to the constructor, and let that constructor do the .. constructing ;)

Further notes:

• g is a terrible name, Group is a much better one (capital G because this function is a constructor)
• Using the jQuery each feels wrong, just go bare metal and use the Array built-ins
• Speaking of which getGroupsWithStatus becomes a one-liner with the Array.filter() built-in
• The reliance of main() on myGroups is unhealthy, you are treating myGroups as a global, it should be local to main, especially if you are putting querying functions within main
• If you are not using a fat arrow syntax function, then you should provide a function name (getGroupsWithStatus = function (status) => getGroupsWithStatus = function queryGroupsWithStatus(status)

This is my counter proposal:

var jsonTmp = '{ "groups": [ { "name": "G1", "status": "inactive" }, { "name": "G2", "status": "active"}, { "name": "G3", "status": "inactive" }] }'

function Group(key, properties) {
    this.key = key;

    properties = properties || {};
    this.name = properties.name;
    this.status = properties.status; 
}
​
function main() {
    this.groups = [];
​
    this.createGroups = function (json) {
      var data = JSON.parse(json)
      this.groups = data.groups.map( (o,key) => new Group(key,o) );   
    }
​
    this.getGroupsWithStatus = function querygetGroupsWithStatus(status) {
      return this.groups.filter( group => group.status == status  )
    }
}
​
var myGame = new main();
myGame.createGroups(jsonTmp);
console.log( myGame.getGroupsWithStatus("inactive") ); 

​

Answer 2

Q1: An array is certainly the most flexible data structure in which to store your data, but this means that any lookup will require a linear search. If you expect to frequently need to look up a group by name, it would probably be better to use a data structure that maps names to objects -- either a simple Javascript object, or an ES6 Map. This will also ensure that names are unique.

So you would initialize:

this.groups = {};

And add groups to it with:

if (typeof myGame.groups[group.name] != 'undefined') {
    // Report duplicate group name error
} else {
    myGame.groups[group.name] = new g(key);
}

Q2: You could have the g() constructor accept all the initial properties, rather than requiring a separate call for each property. To avoid having lots of parameters, you can use ES6 destructuring.

function g({key, name = '', status = 'inactive'}) {
    this.key = key;
    this.name = name;
    this.status = inactive;

    this.setStatus = function (status) {
        this.status = status;
    }

    this.setName = function (name) {
        this.name = name;
    }
}

Q3: That's not a pointer (Javascript doesn't have pointers), it's just a temporary variable. It's certainly a reasonable style, and that's how I would probably write it, to avoid writing myGame.groups[key] repeatedly.

One final note: Don't use a short name like g for a class. The name should be descriptive and memorable. This should be true for all variables and functions, but you can generally get away with short names for local variables that have very narrow scope, such as the iteration variables in loops (e.g. for (i = ...)). The larger the scope of a name, the more important it is that it have a good name. A more appropriate name would be something like UserGroup.