Avoiding collisions of stamp's options argument about stamp-specification HOT 34 CLOSED

I'm not sure this is a good idea. One of the best features of the options object is that it gives the stamp creator more control over the function signature. I think it's OK to have a few reserved properties on the options object. I'm not as sure it's OK to have a few reserved arguments in the stamp function signature.

from stamp-specification.

If you were a TC39 memeber what would you standardize?

from stamp-specification.

I'd stick to the options-object only approach. Additional options are superfluous and there isn't enough motivation to hamper the stamp function signature. There's nothing wrong with a handful of reserved properties, any more than there's anything wrong with a handful of reserved keywords in a programming language.

from stamp-specification.

Are you satisfied enough with my feedback to close this issue? My major concern is that users of the spec are not building stampit. They're building networking libraries, or streaming libraries, or db connection libraries, etc... and they may have need for control over the arguments.

An options object is almost universally useful. However, an instance argument would not be broadly useful across many problem domains. In fact, I've only very rarely wanted the option in Stampit.

stamp is even less universally useful than instance. Why would somebody instantiating a db connection want to pass instance or stamp arguments?

The answer is simple: They wouldn't. However, they may want to use arguments for other things, such as a queue of signals to send across a network connection once the connection has been established...

But they can't do this:

const db = dbStamp({ url, credentials }, ...messageQueue);

Because the stamp is expecting an instance to be the second parameter... so we force them into awkwardness like this?

const db = dbStamp({ url, credentials }, null, null, ...messageQueue);

And in the dbStamp docs the author has to write: "the second and third parameter are not used, so pass nulls instead".

Of course, they could just use the options object for everything, but if certain patterns are common enough, maybe it makes sense to save people some typing by offering a second or third optional parameter. My point is, I don't believe it's our place to dictate function signatures for stamp creators.

from stamp-specification.

Wait a second! Since when we allow any number of parameters to be passed to the factories/stamps?
This is stamp-specification repository, not stampit. For now we allow only one parameter.

If you are talking about oldschool constructors then I should remind you the idea you expressed earlier and we both liked it. - The convertConstructor will be a utility function which implements the specific behavior.

import {convertConstructor} from 'stamp-utilities-module-of-some-kind';
const mqStamp = convertConstructor(MessageQueue); // old school ctor
const queue = mqStamp({args: {MessageQueue: messageQueue}}); // <- the idea is by Eric Elliott

This allows us to compose multiple oldschool constructors. Cool!

You have completely misunderstood my idea, Eric. :)

Please, take a closer look.

1. I did not propose to pass THREE parameters to stamps. But TWO at max.
   Quoting you:

an instance argument would not be broadly useful across many problem domains.

Yes! Yes! Yes!
That's why I proposed it to be the second optional parameter to stamps/factories.
composable([options], [instance]) - third and more arguments will be ignored.

2. <- will type in few hours. I'm busy atm.

from stamp-specification.

Sometimes I think there are two Erics. :)

from stamp-specification.

I think the point I'm trying to make is that currently, the spec is open for extension that would allow additional arguments.

My biggest regret about the Stampit API design was arbitrarily deciding that the first stamp argument would be used to initialize properties. That's usually what I wanted, true, but it wasn't always what I wanted.

Every decision we make about the stamp function signature is a possibility we take away from the stamp author. I think some well-communicated reserved properties of an options-object is an OK compromise.

from stamp-specification.

The stamp's signature is open for extension via the first argument - options.

Take a look at Promises A+.

• The Promise ctor have one parameter - the callback. This will never change. The standard is fixed.
  • The callback is also standardized - it accepts two parameters resolve and reject. This will never change as well, there will be no third parameter ever.
• The .then() is also will never change, although it's got two optional parameters - result and error.
• Same for the .catch() function - single optional parameter error. This is fixed. No second argument will ever be ever standardized.

I mean that I'm strongly against "extending" the composables standard.

The standard must be minimalistic enough. The tools around it must be as comprehensive as possible. If a tool would like to prohibit instance, stamp, args properties in the options object - then let it be.

But the standard must not have any compromises.

Do you rememebr collitions of the stampit v2 static functions? We are going to hit collisions again if we forbid at least tiny portion of options object freedom.

I propose:

1. Standardize two parameters to the stamps. Both optional. Never extend it with the third parameter.
2. If someone would ever need some additional features from the composables - they'd need to implement it as a composable behavior. Basta.

from stamp-specification.

I'd like you to answer the few following questions:

let stamp1 = compose({ init({ instance, stamp }) => {
  console.log(instance);
  console.log(stamp);
}});

What will be printend if in the following case?

stamp1({ instance: { a: 1 }, stamp: { b: 2 } });

And please, take your time to answer this question thoroughly:

Why?

Why the two console.log print such output, but not else?

Thank you.

from stamp-specification.

I can see where @ericelliott is coming from.

@koresar Taking a look at Promises A+
.catch() isn't part of the spec. It exists because using the error param in the .then() is an anti pattern. What kind of hideous API would we have if the signature was .then(error, result)?

It's very easy to mess up multi parameter APIs...objects are much more flexible. There is the possibility of key collision, but it's worth noting that js "classes" also have key collision (constructor) and objects themselves have key collision (prototype). We can't have "no" key collision, so how much do we want to have?

from stamp-specification.

Which idea offers more freedom to stamp authors?

• Dictate only the options argument. Reserve a minimal selection of keys.
• Dictate multiple arguments, reserve no keys. Instead reserve entire arguments

You're looking at an edge case (author wants to use "instance" or "stamp" as an option), and suggesting that we eliminate what may in the future be common case (author wants to utilize more than a single argument for their stamp) in order to enable the edge case.

The work-around if the author wants something like "instance" is to pick a non-reserved key to represent the "instance". This is what JS authors have been doing for years when they name a var args instead of arguments, because arguments was already bound inside the function. I don't think any developer thinks this is an unreasonable compromise. We also understand that we can't call a var Object or Array because those are built-ins.

What you're suggesting is a drastic measure (dictate an entire argument) to fix a problem that is already very small, and already has a simple workaround.

I realize that currently we don't offer any additional arguments, but I'd like the option of changing that in the future if users decide it's something they need.

from stamp-specification.

@JosephClay your single argument proposal does not comply the Eric's comment above. He proposes to have multiple parameters to the stamp function.

But I hear your argument about constructor and prototype and the rest. Good point.

from stamp-specification.

@ericelliott I hear your argument about the edge case, I'd agree it is. Also, I tend to agree that two arguments is somewhat worse than a single options object.

Could you please answer my two questions above? I must know answers now, and you need to add them to the specification text.

Also, I have to ask you to ignore my question less. That'd be very helpful in our future discussions. You tend to avoid straight questions lately. Probably you are very busy with your work.

from stamp-specification.

@JosephClay your single argument proposal does not comply the Eric's comment above. He proposes to have multiple parameters to the stamp function.

I propose that we stick to a single argument for the parameters that we need in order to provide flexibility to stamp authors and users of the stamp specification in the future.

If we use more than one argument, we'll be stuck with the decision going forward. I made the wrong decision in this regard when I created the original Stampit API and decided that the first argument would be properties to assign to the object. That's a mistake I don't want to repeat.

from stamp-specification.

That's a mistake I don't want to repeat.

👍 I do agree. Let's stick with a single ergument.
Though, my two questions worry me much.

from stamp-specification.

The answer to your two questions, according to the current spec should be something like this:

{ a: 1 }
{ [Function: Stamp]
  compose: 
   { [Function: composeMethod]
     methods: {},
     properties: {},
     deepProperties: {},
     initializers: [ [Function] ],
     staticProperties: {},
     propertyDescriptors: {},
     staticPropertyDescriptors: {},
     configuration: {} } }

Reason for the 1st one:

The whole point of the instance parameter is so that users can pass an existing instance of something into the stamp for mutation, so, we use the object passed in as the instance.

Reason for the 2nd one:

When we call the initializer, we ignore the options and simply pass the stamp in as the stamp parameter. In other words, no collision is created. We don't even have to reserve the stamp option. Let me remind you of the proposed options argument for the initializers:

{
  instance,
  stamp,
  options
}

In other words, the initializer call might look something like this:

initializer.call(obj, { instance: obj, stamp: Stamp, options });

• Stamp = the actual stamp function name.
• options = the actual options object that was passed to the stamp function.

I'm not sure what you're worried about. Am I missing something?

from stamp-specification.

If you're satisfied, please close. =)

from stamp-specification.

I do not like the new "options" property we have. It complicates passing data from user to initializers.

let MyStamp = compose({ init({ options }) => { // <- this is Options inside Options
  request(options.requestOptions); // <- this is requestOptions inside Options inside Options
});

MyStamp({ requestOptions });

Options inside Options is a code smell. In my example there are "requestOptions inside Options inside Options" which is even worse.

I am strongly against that third argument.

From the discussion above we all are okay having two reserved properties. Let's drop "options" property from the intializer's options object.

from stamp-specification.

let MyStamp = compose({ init({ requestOptions }) => { // <- this is Options
  request(requestOptions); // <- this is requestOptions inside Options
});

MyStamp({ requestOptions });

from stamp-specification.

Point taken. I've been concentrating on the stamp parameters so far, but as far as I'm concerned, we can pass anything we want to init functions. Whatever is the most convenient:

// Note: simplified - no other stamp to compose with.
// Shouldn't these util functions return stamps?
let MyStamp = init( ({ requestOptions }, instance, stamp) => {
  request(requestOptions);
});

MyStamp({ requestOptions }); // this is the most common case, and doesn't change.

from stamp-specification.

👍

from stamp-specification.

Also, we might want to consider removing the second parameter:

let MyStamp = init( ({ requestOptions }, instance, stamp) => {

It is already part of the first argument:

let MyStamp = init( ({ requestOptions, instance }, stamp) => {

What do you think?

from stamp-specification.

I think, to avoid conflating the options argument passed into the stamp with the init function parameters, maybe this would be better...

let MyStamp = init( ({ requestOptions }, { instance, stamp }) => {

It also provides a clear extensibility point in case we want to add arguments down the line...

let MyStamp = init( ({ requestOptions }, { instance, stamp, arguments }) => {

As we've discussed many times in the past, I think stamp arguments are an anti-pattern, but I thought I'd never allow it into Stampit, and then users demanded it, so I caved. =)

And maybe sometime down the line we'll think of other neat things we could pass into the init functions... I most definitely would prefer we handle that extensibility with an extensible object than a bunch of arguments...

from stamp-specification.

Keep in mind, the stamp instance argument is not necessarily the same object passed into the initializer. For one thing, the argument is optional. For another thing, instances can be replaced by stamps. ;)

from stamp-specification.

That is awfully beautiful idea! Do you want me to create a PR for that?

from stamp-specification.

Sure, that would be great. =)

from stamp-specification.

Oops.. accidentally self-merged this function signature update in the README:

(options, { instance, stamp }) => instance

from stamp-specification.

I'm not yet closing this issue as a reminder for myself to create a PR. Although, this discussion should be considered successfully finished.

from stamp-specification.

Well, before you dump much time into it... I'm conducting an exercise for myself right now -- creating a full implementation of the spec as it is now (including this change) with a full, descriptive unit test suite. Nowhere near complete, currently, but you can monitor my progress in the outstanding PR.

I'm doing this because I'm presenting the spec & example implementation at the WebDirections Conference in Sydney, Australia in October.

If you also want to work on an implementation, that's cool (maybe in the Stampit 3.0 branch?), but I need to do all of this one I'm working on from the ground up so that all of the design implications can really sink in. Does that sound like a fair deal?

from stamp-specification.

Complicated wording. Pardon my English skills. What deal?

from stamp-specification.

I work on the example code for the stamp spec, you work in parallel on the implementation for Stampit 3.0?

from stamp-specification.

Sounds about right.
But we should start form tests regardless.

from stamp-specification.

Right now I'm working on one test at a time:

1. Write the test
2. Watch it fail
3. Implement feature
4. Watch it pass

The thing about TDD is that the tests don't just validate the code. The code also validates the tests. In other words, if you write a test suite with no implementation, there's no way to know whether or not the tests are valid, because they haven't been tested against real code.

from stamp-specification.

I believe we can close this issue now. The README is up to date.

from stamp-specification.