question: remove required new operator

In addition to this:

dataset = new datastore.Dataset({
  projectId: 'my-project',
  keyFilename: '/path/to/keyfile.json'
});

can we support (note the lack of the new keyword):

dataset = datastore.Dataset({
  projectId: 'my-project',
  keyFilename: '/path/to/keyfile.json'
});

We can accomplish this by a technique discussed here.

Essentially detect whether the object calling it is an instance of Dataset or not:

if ( !(this instanceof Dataset) ) {
  return new Dataset();
}

If this change was to be made, I would suggest that developers omit new always and we don't document it as requiring it in the first place as it simplifies what the developer needs to worry about. The capitalization of Dataset implies its a constructor but they can call it just like a regular method on datastore.

Thoughts?

Why?

If their codebase is using JSHint (and probably other linters), they'll have to explicitly turn off the recommendation of capital character referring to something that should be new'd. I'm in favor of continuing to use the new behavior, because hiding it is unnecessary confusion, which only has the purpose of avoiding using "new". I think it's best we stick to the convention and recommend using new since they are instantiating an object.

I don't think it would hurt, however, to add in the if to catch non-instantiating executions.

Yeah, makes sense. I didn't know JSHint would do this.

@rakyll

I've come around. Me, from #168:

It feels like forcing the developer to use new is important for them to understand "you're getting a new object that is meant to be multiply instantiated," but I suppose you can argue that that's an implementation detail of how our internal structure works, and saying gcloud.datastore.dataset(), for example, can be just as clear, especially with the help of the documentation.

I would be interested in hearing what @rakyll and others think about facading our internal use of new, creating an end-user api like:

var gcloud = require('gcloud');
var dataset = gcloud.datastore.dataset({});
var bucket = gcloud.storage.bucket({});

var gcloud = require('gcloud');
var dataset = gcloud.datastore.dataset({});
var bucket = gcloud.bucket({});

😍

Oops, had to update my comment. Hope you still like it:

-var bucket = gcloud.bucket({});
+var bucket = gcloud.storage.bucket({});

Didn't notice that mistake. Yes, that makes more sense and still looks perfect.

feat(app): support global configuration

docs: document apiEndpoint over servicePath (#172)

refactor: drop dependency on lodash.merge and update links (#172)

feat!: drop Node 8 from engines field (#172)

chore(deps): update dependency nyc to v14

chore(deps): update dependency @types/uuid to v8 (#172)

chore: regenerate synth.metadata (#172)

chore(deps): lock file maintenance (#172)

chore: Report warning on `.github/workflows/ci.yaml` (#172)

chore: update jsdoc.js (#172)

Update gcs-resumable-upload to the latest version 🚀 (googleapis#172)

change id vs. name in Cluster (#172)

chore(deps): lock file maintenance (#172)

change id vs. name in Cluster (#172)

change id vs. name in Cluster (googleapis#172)

Navigation Menu

Repository navigation

question: remove required new operator #172

69 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

question: remove required new operator #172

Description

Activity

stephenplusplus commented on Sep 4, 2014

ryanseys commented on Sep 5, 2014

stephenplusplus commented on Sep 6, 2014

ryanseys commented on Sep 6, 2014

stephenplusplus commented on Sep 6, 2014

ryanseys commented on Sep 6, 2014

69 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions