Set and get in the same function

Question

I am attempting to write a function that returns the records that a user has selected on a grid. In this example, I grab all of them and return their IDs.

Grid.prototype.getSelectedRecords = function () {
    var self = this;

     self.selectedRecords = this.grid.selModel.getSelection().map(function (record) {
         return record.internalId;
     });

     return self.selectedRecords;
}

Here I am placing them in a object property AND returning that property. This function, in effect is working as a setter and a getter. Would it confuse other programmers that might be looking for a setter? I don't know if I want to write a setter/getter each time...

With how quirky JavaScript does object-oriented, is this bad?

Answer 1

First of all, I have no idea why you've got self in there. It's unnecessary (and indeed, you didn't even use it in the call to .grid.selModel...)

Secondly: I don't understand the point of setting a property in the first place. You're always returning the result of the grid.selModel... call, so the property isn't necessary. You could just have a local variable - or better yet, just return directly, with no properties or local variables. The method's just a decorator for another method's output.

Now, if the idea is that you can either use someGrid.getSelectedRecords() getter or someGrid.selectedRecords property, then you're going about things in a bad way.

If I see that I can use someGrid.selectedRecords directly, then no, it should not be overwritten if I call someGrid.getSelectedRecords(). That makes no sense. In fact, it'll cause something like this:

someGrid.getSelectedRecords() == someGrid.selectedRecords // true
someGrid.selectedRecords == someGrid.getSelectedRecords() // false

Wat?

So to directly answer your question: No, don't do what you're doing in that piece of code. But the reason why you shouldn't is because using both accessor methods and direct property access is a bad approach to begin with.

If you use getter and setter methods, your properties should be off-limits to external code. If you only use properties, you shouldn't add getter or setter methods, as they confuse the API greatly.

You could conceivably use Object.defineProperty() which'll let you, well, define a property but with explicit getter and setter functions (if the target runtime(s) support it). But frankly it's easier to just write getter and setter functions if you want to maintain a barrier between your object's instance data and the outside world.

But, again, in this particular case, your object doesn't seem to need any instance data at all as it just delegates to something else. This should work equally well, without getting properties mixed up in things:

Grid.prototype.getSelectedRecords = function () {
  return this.grid.selModel.getSelection().map(function (record) {
    return record.internalId;
  });
}

Returning the result of map and not storing has the added benefit of never returning a reference to instance data. The function will always return a brand new array.

Now, you might still want to memoize/cache those IDs, but in that case you'll be better served with "private" closure variables, rather than inherently "public" properties.

You could also go the opposite route and only use naked properties, and rely on users to use them correctly, or reap what they sow if they don't.
But that doesn't really apply here, since the method in question a shortcut for another object's method.

I don't know if I want to write a setter/getter each time...

I can sympathize, but depending on what you're trying to accomplish you might have to.