dresende / node-orm2

Object Relational Mapping
http://github.com/dresende/node-orm2
MIT License
3.07k stars 379 forks source link

Model class should use prototype rather than assigning new methods to each model in the constructor. #773

Open insidewhy opened 7 years ago

insidewhy commented 7 years ago

Using prototype has a number of advantages, but mostly the current method being used to populate methods per instance is very bad for performance and memory usage.

insidewhy commented 7 years ago

It's the same for other classes too, like AggregateFunctions etc. In fact the only place this code is using prototype is in the drivers. 👎

dxg commented 7 years ago

Probably has something todo with the fact we're using getters and setters. Perhaps when this package was originally written JS didn't allow getters/setters on prototypes? Not sure.

Would be interested to see if it's possible to change it. Feel free to submit some code.

insidewhy commented 7 years ago

Even if that were the case it's still a terrible reason for making every non-property instantiated in the constructor. This is some of the worst performing code I've used in ages, it was a massive mistake for my previous client to use it. No one should use orm2.