An opportunity to clean up some legacy API stuff

I posted this as an Issue in can-define but I bring it up here also to encourage a discussion.

https://github.com/canjs/can-define/issues/21

I find the Map.extend signature and ones like it odd, where you have to move arguments around depending on how many you provide. I don’t think this is a common idiom in JS, and with ES6+ enhancements to the language it’s starting to feel very wrong.

I ALSO don’t like overloading functions, where different actions are taken depending on the type of argument is passed: like in jQuery if you pass $() a function it’s invoked on DomReady, if you pass it a string selector it finds and wraps dom from the document, and if you pass it a string of HTML it creates DOM nodes.

I think overloading function like that is a relic of a pre-es6 module system where you could do like

import { querySelector, createDomNode, onDomReady } from 'jquery'; 

Anyway… I think we do a lot of that around CanJS too and maybe we could take this opportunity to embrace ES6 modularity and clean that up?

Thoughts? Flames? Props?

2 Likes

Can you explain which ES enhancements make it feel wrong?

It’s fairly common in JS (jquery does it).

Structurally in other languages, the name of the class, static properties, and then instance properties are described in that order. This makes sense as its ordered “most important first”. But defining static properties is less common. This is why it works the way it does. It would be odd to have the name not co-located with the variable accepting the return value.

In 3.0, generally speaking, people will less often need to define static properties now that can.Model is deprecated. So this will be an occasional nuisance.

Finally and most importantly, 3.0 is not about API changes. It’s about structural changes that required some API changes. We want it as easy to upgrade to as possible. So as this doesn’t have a huge payoff and would create a decent amount of breaking (though easily fixed) changes, I’m not inclined to support it for 3.0.

However, I’d be happy to keep the issue open in can-construct for 4.0 discussion.

I agree that this is a point of confusion for our APIs. I always forget which comes first and have to look it up, I think if additional arguments were appended it would be easier to remember/understand.

Anyways, you should be able to use regular constructors/classes with can-define and not use the can-define/map/ API at all like:

class Something {

}

define(Something.prototype, {
  foo: "string"
});

This feels a bit better to me. If/when decorators happen it could be cleaned up even more.

1 Like

What I meant was, specifically regarding method overloading to be clear, that with imports being a language thing and not a library thing, it seems more reasonable to provide three different methods/functions rather than overload one function with 3 different behaviours depending on input.

I should have probably made two topics as moving arguments around is related-but-distinct from method overloading.

Method overloading is common in JavaScript but I think will soon be considered an anti-pattern now that imports provide an (arguably) clearer semantic.

Changing parameters based on arity, aka moving arguments around depending on how many you pass, is not common, or at least I think it is not common, I can’t think of many/any examples/

True, but can-define is a new feature/library, so it’s not really a breaking change yet.

I dream of this day:

@observeable
class Something extends SomethingElse {
  // whatever
}

// OR

class ThisOtherThing {
   @observable
   quantity: 0,

   @observable
   cost: 9.99

   @observable
   total() {
      this.count + this.cost;
   }
}

But being consistent with can.Construct is the point. Right. Ignore me.

Why does that seem reasonable? What would that look like?

import {prototypeExtend} from "can-construct";
import {nameExtend} from "can-construct";
import {staticExtend} from "can-construct";
import {staticAndPrototypeExtend} from "can-construct";
import {nameAndPrototypeExtend} from "can-construct";
import {nameAndStaticExtend} from "can-construct";
import {nameAndStaticAndPrototypeExtend} from "can-construct";

I don’t think it’s better to provide different methods functions than one function. I think jQuery, the most popular JS library of all time, proved that in a “non intellisense-able” language, it’s better (less cognitive load on a beginner developer) to have a fewer number of functions that are overloaded than a larger number of functions. Hence:

.attr()
.attr(prop)
.attr(props)
.attr(prop, value)
.attr(prop, function)

instead of:

.getAll()
.get(prop)
.setAll(props)
.set(prop, value)
.setFunc(prop, function)

Also, I’m not sure a module format that hasn’t landed in most browsers, that most people won’t be using for at least 2 years, should be a large factor in how we weigh API design.

That first example seems like an over exaggeration, but your second example with .attr() has made me think more, as I am not sure where I think the line is with method overloading.

I am so used to the whole 1 argument is getter, 2 argument is setter thing, that I am not sure if a “feel” like
.get() and .set() are better, but trying to think objectively they do have very different functions, so should actually be better by my premise/reasoning, so though I am not sure, I’ll try to take the position for arguments sake.

I do on the other hand feel that the whole “A string for one property and an object for multiple properties” on a setter is maybe jsut a convention we’re used to, and maybe not worth the variation, I mean why not always an object.

.set('someProp', value)
.set({ someProp: value })

as for getter, it feels fine that a getter would give you all props with no args, or you can “limit” the result with a string, like a psuedo key.

.get() // { someProp: true }
.get( 'someProp' ) // true

But the last one, where set props with a function, yeah, to me that seems like maybe thats different enough to be a separate method.

.setWithFunc( someFunctionThatReturnsAnObject )

As for:

Also, I’m not sure a module format that hasn’t landed in most browsers, that most people won’t be using for at least 2 years, should be a large factor in how we weigh API design.

I can totally get behind this, so yeah, now is maybe not the time to start removing overloading from our APIs. But I do think the other thing, where the argument order changes depending on how many arguments are passed, is worth changing sooner than waiting.

So okay, regarding method overloading, this is now purely a philosophical discussion then, with no action necessary, but I do think we’ll see this marked as an anti-pattern in the future.

Eh? Eh? Moar “intellisense-able” now though isn’t it. :slight_smile: