Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS client: Model methods should be added to prototype #2044

Closed
delenius opened this issue Feb 4, 2016 · 1 comment
Closed

JS client: Model methods should be added to prototype #2044

delenius opened this issue Feb 4, 2016 · 1 comment

Comments

@delenius
Copy link
Contributor

delenius commented Feb 4, 2016

In the generated model classes, each instance contains the methods constructFromObject, toJson, plus getters and setters for all the fields. Currently, these methods are added as anonymous functions to each instance. Meaning, if I have 100 instances of some class, I have 100 copies of each of the methods.

These should be added to the class's prototype instead. For example, instead of (current version)

var Person = function Person(name) {
  var self = this;
  self['name'] = name;
  self['age'] = age;
  self.constructFromObject = function(data) {
      if (!data) {
        return this;
      }
      self['name'] = ApiClient.convertToType(data['name'], 'String');
      return this;
  }
  self.getName = function() {
    return self['name'];
  }
  self.setName = function(name) {
    self['name'] = name;
  }
  self.toJson = function() {
    return JSON.stringify(self);
  }
}

it should generate (new version):

var Person = function Person(name) {
  this['name'] = name;
}

Person.prototype.constructFromObject = function(data) {
  if (!data) {
    return this;
  }
  this['name'] = ApiClient.convertToType(data['name'], 'String');
  return this;
}

Person.prototype.getName = function() {
  return this['name'];
}

Person.prototype.setName = function(name) {
  this['name'] = name;
}

Person.prototype.toJson = function() {
  return JSON.stringify(this);
}

Two additional comments:

  1. It would be nice to be able to omit the getters and setters entirely, as they cause a lot of bloat that is not desired in many cases.
  2. The toJson prototype is the same for every model, and could be moved to a higher level prototype:
var ApiModel = function(){}
ApiModel.prototype.toJson = function() { return JSON.stringify(this); }

var Person = function Person(name) {
  ApiModel.call(this);
  this['name'] = name;
}

// all the Person prototype function follow like before, except toJson
delenius pushed a commit to delenius/swagger-codegen that referenced this issue Feb 6, 2016
@delenius
Copy link
Contributor Author

delenius commented Feb 6, 2016

I submitted a PR. Note that it does not deal with the two "additional comments" above.

delenius pushed a commit to delenius/swagger-codegen that referenced this issue Feb 6, 2016
delenius added a commit to delenius/swagger-codegen that referenced this issue Feb 6, 2016
delenius added a commit to delenius/swagger-codegen that referenced this issue Feb 8, 2016
This is just the result of running ./bin/javascript-petstore.sh
after the fix for swagger-api#2044.
delenius added a commit to delenius/swagger-codegen that referenced this issue Feb 8, 2016
Fixes "additional comment swagger-api#2" in swagger-api#2044.

Saves memory by not repeating the `toJson` method in every model
class.

The new `ApiModel` base class may also be useful for other purposes
in the future.
delenius added a commit to delenius/swagger-codegen that referenced this issue Feb 9, 2016
Fixes "additional comment swagger-api#2" in swagger-api#2044.

Saves memory by not repeating the `toJson` method in every model
class.

The new `ApiModel` base class may also be useful for other purposes
in the future.
delenius added a commit to delenius/swagger-codegen that referenced this issue Feb 12, 2016
Fixes "additional comment swagger-api#2" in swagger-api#2044.

Saves memory by not repeating the `toJson` method in every model
class.

The new `ApiModel` base class may also be useful for other purposes
in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants