Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

single behavior for accessing any instance property #3569

Closed
taburetkin opened this issue Dec 15, 2017 · 7 comments
Closed

single behavior for accessing any instance property #3569

taburetkin opened this issue Dec 15, 2017 · 7 comments

Comments

@taburetkin
Copy link
Contributor

Sometimes i just do not remember can i define a property as function or not.

that`s because of different behavior in different cases.
sometimes its possible, sometimes its not.

i want to offer little improvement for resolving this "issue"

  1. first of all, getOption always returns raw value
    so, if i have an option as function i have to wrap it by function invoker if i need the result of that function.
  2. in second, _.result do not accepts arguments, only default value for empty result.
  3. in third _.result do not handle constructors, and i think its possible to handle it, atleast for known constructors

so, here is my improvement

var SmartGetOptionMixin = {
	getOption:function(key, force){
		var mnGetOption = Mn.Object.prototype.getOption;
		var firstResult = mnGetOption.call(this, key);
		if(!force || typeof firstResult !== 'function') return firstResult;
		return _result(this, firstResult);
	}
}

function _result = function(){
	var args = [].slice.call(arguments);
	var context = args.shift();
	var key = args.shift();
	var firstResult = typeof key !== 'object' ? key : context[key];
	if(!_.isFunction(firstResult) || isKnownCtor(firstResult)) return firstResult;
	return firstResult.apply(context, args);
}

var Bb = Backbone;
var knownCtors = [Bb.Model, Bb.Collection, Bb.View, Mn.Behavior, Mn.Object];
function isKnownCtor(arg){
	if(!_.isFunction(arg)) return false;
	return _(knownCtors).some(function(ctor){
		return arg === ctor || arg.prototype instanceof ctor;
	});
}

i offer to extend getOption and if an user needs some smart behavior he can pass force attribute
this will return value or class
and by default getOption will work as today

this can help to pass any property as value, definition or a function
and the behavior will be same everywhere.

@paulfalgout
Copy link
Member

I'm 👎 on this. I think the use of getOption should be limited and don't believe extending it's functionality is a good idea for Mn. getOption does one simple cheap thing and that's better than a single function that does all the things.

@taburetkin
Copy link
Contributor Author

taburetkin commented Dec 16, 2017

in defense of my point i would like to offer this example:

const Test = Mn.View.extend({
    ViewClass: function(){
        //assume we allow
        //1. override this by options
        //2. be a ViewClass or be a function returning a ViewClass
    },
    runTest(){
        var MyViewClass;
        //current way to resolve this
        var value= this.getOption('ViewClass');
        if(_.isFunction(value) && (value.prorotype instanceof Backbone.View || value ===  Backbone.View))
            MyViewClass = value;
        else if(_.isFunction(value))
            MyViewClass = value.call(this);

        //with smart getOption.
        MyViewClass = this.getOption('ViewClass',true);

    }
});

maybe getOption is not a good name for this, but i very like the idea that with getOption options has higher priority than property of an instance.
this allow easily to override property if it need.
and the main goal was to save ability get option or property with more smart behavior under the hood

@paulfalgout
Copy link
Member

paulfalgout commented Dec 16, 2017

The reason why getOption was in general pulled from internal use is this:

var MyThing = Mn.Thing.extend({
  foo: 'bar',
  setFoo(val) {
    this.foo =val;
  },
  doFoo() {
     this.gizmo.do(this.getOption('foo'));
  }
});

var myThing = new MyThing({ foo: 'baz' });
myThing.doFoo(); // did with 'baz'
myThing.setFoo('bazinga');
myThing.doFoo(); // still done with 'baz' because nothing can override the option.
myThing.foo = 'still no change';
myThing.doFoo(); :-/

@taburetkin
Copy link
Contributor Author

taburetkin commented Dec 16, 2017

hm...
there should be getProperty(key) :))
and it should work as property first, then option
and there will be two methods: getProperty and getOption ))

and they both can be smart, of course
and mergeOptions with getProperty will allow to override property by option and will allow to have smart getter

@paulfalgout
Copy link
Member

I would lean towards removing getOption altogether before adding getProperty

@taburetkin
Copy link
Contributor Author

taburetkin commented Dec 16, 2017

utils/smart-get-property.js

import _ from 'underscore';
import isKnownCtor from './is-known-ctor';
export default function(key, force = true, ...args){
	if(key == null) return;

	let value = this[key];

	if(value === undefined && typeof this.options === 'object')
		value = this.options[key];
	
	if(!_.isFunction(value) || isKnownCtor(value)) return value;
	
	return force ? value.apply(this, args) : value;

}

utils/smart-get-option.js

import _ from 'underscore';
import isKnownCtor from './is-known-ctor';
export default function(key, force = true, ...args){

	if(key == null) return;

	let options = _.result(this, 'options');

	let value = typeof options === 'object' && options[key];

	if(value === undefined)
		value = this[key];

	if(!_.isFunction(value) || isKnownCtor(value)) return value;
	
	return force ? value.apply(this, args) : value;
}

utils/is-know-ctor.js

import _ from 'underscore';
import Bb from 'backbone';
import Mn from 'backbone.marionette';

const knownCtors = [Bb.Model, Bb.Collection, Bb.View, Mn.Behavior, Mn.Object];

export default (arg) => {
	return _.isFunction(arg) && _(knownCtors).some(function(ctor){
		return arg == ctor || arg.prototype instanceof ctor;
	});
}

:)

@paulfalgout
Copy link
Member

I'm strongly against adding this to Mn core. It can be provided as a 3rd party plugin for anyone interested in incorporating these features. If the community and/or @marionettejs/marionette-core feels strongly otherwise, we can reopen for further discussion.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants