CODEWITHSUNDEEP
javascript
Ju Ba
Ju Ba

Reputation: 11

Is this a bad way to construct an object inside of an object?

I am using this code to make VN.CH.Mister1 using the VN.CH.Make() function inside the VN.CH object but feel like it is weird and inefficient and have been unable to find a more effective way of doing it, preferably all inside the object, while looking online.

let VN = {
    CH: {
        Make: function(name) {
            this.Name = name;
            this.Exists = 1;
        }
    }
};

VN.CH.Mister1 = new VN.CH.Make('Mister1');

If anyone could help explain a more effective way of doing this, or a way to do it all inside of the VN.CH object or point out any misunderstandings I might have about Javascript in general from what you see here I would greatly appreciate it.

Upvotes: 1

Views: 71

Answers (3)

FK82
FK82

Reputation: 5075

Seems like you want CH to manage some kind of collection of Person objects (i.e. the kind of object that VN.CH.Make creates). So, instead of writing

VN.CH.Mister1 = new VN.CH.Make('Mister1');

you could put a constructor function Make on VN.CH and write a function VN.CH.make which adds new instances.

CODE:

const VN = {
  CH: {
    Make: function Make(name) {
      this.name = name;
      this.exists = 1;
    },
    make(name) {
      this[name] = new this.Make(name);
    }
  }
};
    
VN.CH.make('Mister1');

console.log(VN.CH.Mister1);

Ultimately however, I think it's clearer if you abstract your code by creating two classes with separate concerns:

  • PersonRegistry which creates Person instances and manages them; and,
  • Person which represents a person with a name and exists property

and then just make VN.CH point to an instance of PersonRegistry:

CODE:

class Person {
  constructor(name) {
    this.name = name;
    this.exists = 1;
  }
}

class PersonRegistry {
  constructor() {
    this.instances = {};
  }
  
  make(name) {
    this.instances[name] = new Person(name);
  }
  
  get(name) {
    return this.instances[name] || null;
  }
}

const VN = {
    CH: new PersonRegistry()
};

VN.CH.make('Lady1');
VN.CH.make('Mister1');
VN.CH.make('LadyA');
VN.CH.make('MisterA');

console.log(VN.CH.get('Lady1'));
console.log(VN.CH.get('Mister1'));
console.log(VN.CH.get('LadyA'));
console.log(VN.CH.get('MisterA'));

Note that you actually don't need the class declarations (you could just as well work with object literals); but, working with classes expresses your intentions more clearly imo and you can add subclasses later if needed.

Upvotes: 0

T.J. Crowder
T.J. Crowder

Reputation: 1074575

If you need the Make constructor and it has to be on vn.ch, what you have is fine other than using non-standard naming. Obviously you can do what you like in your own code, and there is sometimes call to make an exception, but overwhelmingly the naming standards in JavaScript are that variables and properties start with a lower-case letter, so:

let vn = {
    ch: {
        Make: function(name) {
            this.name = name;
            this.exists = 1;
        }
    }
};

vn.ch.mister1 = new vn.ch.Make('Mister1');

If you don't need the Make constructor, you can do it more simply:

let vn = {
    ch: {
        mister1: {
            name: "Mister1",
            exists: 1
        }
    }
};

If the Make constructor doesn't have to be on vn.ch, then:

function Make(name) {
    this.name = name;
    this.exists = 1;
}

let vn = {
    ch: {
        mister1: new Make("Mister1")
    }
};

Upvotes: 1

Willem van der Veen
Willem van der Veen

Reputation: 36630

I would separate the concerns of object creation and the place where the objects are stored. Here is an example of how you could achieve this (among many other ways):

let peopleObj = {}  // Here is where you objects life

function make (name) {   // Here is where you create them
  this.name = name;
  this.exists = 1;
}

peopleObj.Mister1 = new make('Mister1');
peopleObj.Mister2 = new make('Mister2');

console.log(peopleObj);

Upvotes: 0

Related Questions