stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 102 forks source link

Recommended approach for private data #311

Closed far-blue closed 7 years ago

far-blue commented 7 years ago

Hi :)

Is the recommended approach for private data to create a closure that returns an object with the methods you want, which you execute and pass to .methods() or the methods: property and within which you hold private vars?

e.g.

let Foo = stampit.compose({
  methods: function () {
    let privateVar = 'baa';
    return {
      getPrivateVar () {
        return privateVar;
      }
    }
  }()
})

Or is there a neater way?

koresar commented 7 years ago

Yeah, there is a neater way. :)

There is a whole new ecosystem of stamps - https://www.npmjs.com/~stamp One of them is https://www.npmjs.com/package/@stamp/privatize

It was first introduced in Fun with stamps. Episode 12. But it's more like Java "protected", rather than "private". The Episode 13 explains how to make it more private.

If you have any questions regarding the @stamp/privatize I'd recommend asking about them here - https://github.com/stampit-org/stamp

There are few other ways to privatize properties in JS:

See your question answered here - https://github.com/stampit-org/stampit/issues/229

far-blue commented 7 years ago

that's great, thanks :) I'm starting to understand how all the bits fit together now!

koresar commented 7 years ago

amazing! :) I myself learning how to properly educate people about stamps. Your example is vivid. So, thank you!

ericelliott commented 7 years ago

I use closures in init functions for data privacy in stamps and functional mixins in general. It is the technique I recommend, and I find it much cleaner than privatize.

danielkcz commented 7 years ago

Just want to share a neat way of adding methods within initializer.

compose({
  init(opts, { stamp }) {
     const { myPrivateVar } = stamp.compose.configuration
     Object.assign(this, {
       myMethod() { return myPrivateVar + opts.passedVar }
     })
  }
})

I am thinking that it would be lovely being able to avoid the use of Object.assign, but keeping the simplicity of the solution. For example returning an object that gets assigned to the instance methods. Otherwise, if you want a method to have its proper display name, you have to do this.myMethod = function myMethod() or const myMythod = () => {} followed by this.myMethod = myMethod. Going simply with ˙this.myMethod = () => {}˙ doesn't work.

It could be an actual candidate for a stamp specification grow. It's not possible to provide such solution with a stamp, am I right?

koresar commented 7 years ago

Stampit v1 had the extend (aka Object.assign) method inbuilt - stampit.extend. Its main purpose was exactly as your code describes @FredyC.

danielkcz commented 7 years ago

@koresar I am not sure how that's relevant. It's surely great there was some possibly similar feature, but it's not there now. Can you express your opinion on the idea instead? :)

ericelliott commented 7 years ago

Hi @FredyC -- I'm not sure what you're getting at. JavaScript already supplies shortcuts for object composition, which is why we removed extend. Consider this form, coming soon to the JS spec. This already works in babel:

init((
  {passedVar},
  { instance, stamp }
) => {
  const { myPrivateVar } = stamp.compose.configuration;
  return {
    ...instance,
    myMethod() { return myPrivateVar + opts.passedVar }
  };
});
koresar commented 7 years ago

Oh! I think I get it!!! @FredyC please supply code examples next time. :) But I think this is what you are thinking of:

const Stamp = stampit({
  init: function () {
    return { bar: 1 }; // THIS OVERWRITE OBJECT ATM
  },
  methods: {
    foo() {}
  }
});

const obj = Stamp();

assert(Object.getPrototypeOf(obj) === Stamp.compose.methods);
assert(typeof obj.foo === 'function');
assert(obj.bar === 1);

That would be a breaking change to the specification. How many people would benefit form that?

danielkcz commented 7 years ago

Ok, I guess that @ericelliott comment is closer to what I meant. Although I am not sure that using ...instance is a good idea because some people may be using eg. Symbols stored on the instance and with this, it would be lost.

compose({
  privateInit(opts, { stamp }) {
     const { myPrivateVar } = stamp.compose.configuration
     return {
       myMethod() { return myPrivateVar + opts.passedVar }
     }
  }
})

It's the same signature as an initializer, but different name (WIP) so it can be used only to return new methods that would be added to the stamp. It's just an idea, probably not that critical.

koresar commented 7 years ago

@FredyC JFYI. The ...instance is a shortcut of Object.assign(). And it copies both string properties and symbol properties. See MDN.

danielkcz commented 7 years ago

Ah ok, wasn't aware of that. I still don't particularly like this feature of returning a different instance object, but I see that you don't like my idea, so let's forget about it :)

ericelliott commented 7 years ago

We could pass an extend utility fixed to the instance into init functions (which are basically functional mixins....) then you could do:

init((
  {passedVar},
  { instance, stamp, extend }
) => {
  const { myPrivateVar } = stamp.compose.configuration;
  return extend({
    myMethod() { return myPrivateVar + opts.passedVar }
  });
});

It would be trivial to define the fixed extend like this:

const instance = { a: 'a' };
const extend = Object.assign.bind(null, instance);

extend({b: 'b'}); // {a: 'a', b: 'b'}

Doing so would not be a breaking change.

danielkcz commented 7 years ago

@ericelliott That sounds like a good idea. Actually wouldn't it be nicer if stamps would allow customizing that second object that's passed into initializer? That way we could add this functionality as another composable stamp instead of hard coding there. I am not sure how to call that yet, but it could be similar to configuration and deep merged there when an instance is created.

ericelliott commented 7 years ago

Stamps really shouldn't be depending on the existence of other stamps. If you have to mix a stamp in to get extend passed in, you introduce an implicit dependency on that stamp for every stamp that uses extend.

I'm really not sure this feature is needed, considering that {...instance, new: stuff}} is already on the standards track. IMO, best practice should be to favor object spread going forward. Which means that stamps should not count on extending an existing object, because other stamps may be returning a completely new object. =)

koresar commented 7 years ago

Actually wouldn't it be nicer if stamps would allow customizing that second object that's passed into initializer?

it's already there. Called "configuration". Just use stamp.compose.configuration for all the new stuff.

init((
  {passedVar},
  { instance, stamp }
) => {
  const { myPrivateVar, extend } = stamp.compose.configuration;
  return extend(instance, {
    myMethod() { return myPrivateVar + opts.passedVar }
  });
});