Extending instance with other stamps on fly?

[danielkcz]

It feels little but strange that you can essentially add methods, statics and stuff in initializer, but you cannot actually compose any further. Even the fact that you have access to compose method itself and it does nothing might confuse some people.

function initializeNodeModel(nodeInstance, { stamp }) {
    stamp.compose(
        Getter('nodeInstance', nodeInstance)
    );
}

I tried also this, which might have been working, but it wont copy my Symbol properties (#86), so not much useful either.

function initializeNodeModel(nodeInstance, { instance, stamp }) {
    return stamp.compose(
        instance,
        Getter('behaviorNode', nodeInstance)
    ).create();
}

Maybe after adding support for Symbol this might work, but still it's bit insane that you have instance and you throw it away to make a new one. Perhaps I should just rethink the way I am doing this in here...

[koresar]

I'm not quite sure what you want to achieve. :) Looking at the code I'm pretty sure you are not utilizing a feature you actually need. May you want to check "statics", "propertyDescriptors" and "configuration"?

The example below might be useful:

const Getter = stampit().statics({
  getter(propName) {
    // `this` references the stamp itself
    return this.init(function () { // add new initializer and return a new stamp
      // `this` references the object instance
      Object.defineProperty(this, propName, { get: ...
    });
  }
});

const MyStampWithGetter = MyStamp.compose(Getter).getter('nodeInstance');

Alternatively, you can utilize propertyDescriptors feature of stampit v3.

const MyStampWithGetter = MyStamp.compose({
  propertyDescriptors: {
    'nodeInstance': { get: ...
  }
});

Or even better, utilize both statics and propertyDescriptors:

const Getter = stampit().statics({
  getter(propName) {
    // `this` references the stamp itself
    return this.compose({ // add new ropertyDescriptor and return a new stamp
      propertyDescriptors: {
        `${propName}`: { get: ...
      }
    });
  }
});

const MyStampWithGetter = MyStamp.compose(Getter).getter('nodeInstance');

[koresar]

On the other hand, using getters in JavaScript is a bad idea IMO, - the object becomes unpredictable. I would recommend you to utilize private data and public getSomething methods:

const Getter = stampit().statics({ // adding static properties to Stamps
  getter(propName, propValue) { // adding method 'getter' which accepts two args.
    return this.methods({ // add new method and return a new stamp
      `get${_.capitalize(propName)}`: () => propValue
    });
  }
});

const MyStampWithGetter = MyStamp.compose(Getter).getter('nodeInstance', nodeInstance);
const obj = MyStampWithGetter();
obj.getNodeInstance(); // getter method

[danielkcz]

Well I see you haven't really looked at the code of my Model yet :) It uses that last approach of yours and storing getter values to the Symbol. https://gist.github.com/FredyC/3a1bac834c971f604b8bfac4a6f813c4

In this case I simply wanted to add another getter based on value received during initialization. Thing is that nodeInstance is something I wrapping to the stamp, it has simple properties like name and description. So I thought if I just pass whole instance as options object, it would use its values for my getters. However I needed also to store whole object. Well I ended up with this the end:

import { Model, Getter, Statics } from './Model';

const NodeModel = Model('Node').compose(
    Getter('name'),
    Getter('description'),
    Getter('nodeInstance'),
    Statics({ buildFromInstance })
);

export default NodeModel;

function buildFromInstance(nodeInstance) {
    return NodeModel.create({
        nodeInstance,
        name: nodeInstance.name,
        description: nodeInstance.description,
    });
}

Either way as I said in a first paragraph, I find it bit strange that you can add methods using the stamp argument in initializer, but calling compose does nothing.

[koresar]

Calling .compose always creates a new stamp. It does not mutate an existing stamp.

What I find weird is Getter and Statics and Model. These are factory functions. Stamps themselves are factory functions. So, you're wrapping factory functions into factory functions. Whereas I propose to not create any wrapper factory functions at all, but utilize Stamps for that.

You can simply implement getters with propertyDescriptors, no symbols needed for that. Also, you need to really take a deeper look at the configuration feature. It's powerful. And staticProperties. These three together will allow you to avoid wrapping factories into factories.

But if you are happy with what you have - then feel free to proceed. :)

[danielkcz]

Well, the configuration is still quite foggy for me, I don't understand what is it good for and how it is used. The stampit code is too advanced for me I guess, because I just cannot spot how is that used in there.

Isn't result of every stamp a factory function? That's kinda the way how stamps are supposed to be working, composing each other to create something meaningful, right?

I will look into propertyDescriptors, thanks.

[koresar]

Stamps are factory functions. This means the stamps create objects: let obj = Stamp(...);

Composing stamps with stamps (or POJO descriptors) always creates a new stamp. Nothing else.

If compare with classes: const Stamp = compose({ ... }) ~ class Class { ... } obj = Stamp(...) ~ obj = new Class(...) const Stamp = compose(S1, S2, S3) ~ class Class extends S1, S2, S3 {...}

[danielkcz]

Ok I think understand what you mean now. The Getter, Setter, etc... are not stamps. I would not call them factory functions either. They are just wrappers for having closure and this wrapper creates a stamp. Essentially what you are proposing is something like this, correct?

const NodeModel = Model('Node')
    .getter('name')
    .getter('description')
    .getter('nodeInstance')
    .statics({ buildFromInstance })
);

I've decided I don't like such chaining approach. It creates a stamp in every separate step while throwing away previous one. This is very bad for memory sensitive use cases when you cannot afford to have a garbage collector do much of unnecessary work. For my approach I am getting one stamp created at single step.

Also as I explained in gitter, I cannot reuse that getter to make read/write property too.

[koresar]

Ah, right. Good it works for you. ☺️

Here goes my favourite quote: Premature optimization is the root of all evil.

Cheers mate

On Thu, 19 May 2016, 16:45 Daniel K. notifications@github.com wrote:

Ok I think understand what you mean now. The Getter, Setter, etc... are not stamps. I would not call them factory functions either. They are just wrappers for having closure and this wrapper creates a stamp. Essentially what you are proposing is something like this, correct?

const NodeModel = Model('Node')

.getter('name') .getter('description') .getter('nodeInstance') .statics({ buildFromInstance }) );

I've decided I don't like such chaining approach. It creates a stamp in every separate step while throwing away previous one. For my approach I am getting one stamp created at single step.

Also as I explained in gitter, I cannot reuse that getter to make read/write property too.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/stampit-org/stamp-specification/issues/88#issuecomment-220241029

[danielkcz]

Yea, there two sides of that coin. Premature optimization is bad, but trying to deliberately lower performance while you know you can do better without sacrificing maintainability isn't also brightest.

https://medium.com/@DanShappir/the-benefits-of-javascript-premature-optimization-ee3b842c42c8#.mm6czfxzn

[koresar]

Don't know what's your situation, but when I compose stamps I do that once on the application startup. After that, I just create objects from the stamps. So, there is no performance loss.

And the optimization you have done here will save you about 100 nano seconds.

97% of the time the performance suffers because of I/O like graphics, networking, file system, etc.

You have fixed something which is not your bottleneck in my opinion.

[danielkcz]

Yea you are right, I am trying to get rid of this nasty habit of optimizing, but it's tough :) I will think about your approach more deeply soon, thank you.

[danielkcz]

@koresar Ok I gave this another try and I ended up with this. I guess it's really better and I even managed to have a same code for getter and setter although I had to use the Symbol again for storing value on the instance.

const NodeModel = Model('Node')
    .getter('id')
    .property('name')
);

const node = NodeModel({ id: 'mynode' });
node.getId() === 'mynode';

I did consider your example you shared with me on Gitter, but it has one serious weakness, that map passed in is shared by all stamp instances, it's not separate for each instance.

const Getter = stampit().statics({
  getter(map) {
    const methods = {};
    Object.keys(map).forEach(key => {
      methods[`get${_.capitalize(key)}`] = () => map[key]; // return reference
    });
    return this.methods(methods);
  }
});

const Setter = Getter.statics({ // inherit from Getter stamp
  getter(map) {
    const methods = {};
    Object.keys(map).forEach(key => {
      methods[`set${_.capitalize(key)}`] = newValue => { map[key] = newValue; }
    });
    return this.getter(map).methods(methods); // using inherited 'getter' method
  }
});

const MyStampWithGetSet = MyStamp.compose(Setter).setter({description: 'bla bla'});
const obj = MyStampWithGetSet();
obj.getDescription(); // bla bla
obj.setDescription('foo foo');
obj.getDescription(); // foo foo

[koresar]

But you can always clone the object inside the setter(map):

map = Object.assign({}, map);

to make sure the map is not shared.

I assume, you had to use Symbol is order to have "protected" (or shared between stamps) state. There was a proposal to share state between stamps: https://github.com/stampit-org/stamp-specification/issues/84 But we decided not to implement for good.

[danielkcz]

But you can always clone the object inside the setter(map):

How is that gonna help? If I do it in the setter, it's still variable per stamp and not per instance since it's static method.

I had to use Symbol not because of protected behavior, but because of the same issue there with this map variable. When I have stored value simply in a closure, it became shared by all instances.

[koresar]

Oops. That was my late night delusion. :) I'll get back to this later (it's Monday morning here).