backbone-forms
backbone-forms copied to clipboard
Suggestion getValue() on Select editor to parseInt
Hi I'd like to make a recommendation to the Select editors getValue() method. At current when using an "id" of a model say, it will return a string from the select list which is fine when using it in conjunction with parseInt for handling parent dependant select lists(like country - region). But having to always parse the value to an Int seems to me like a code smell.
I propose changing the getValue() method to something that will parse an Integer if possible otherwise returning the normal this$el.val(). Which will cut down the need to parseInt when checking a select lists value against say constants for handling other form events like hiding or showing other form inputs.
Thoughts?
getValue: function() {
if _.isNaN(parseInt(this.$el.val())) {
return this.$el.val()
} else {
return parseInt(this.$el.val())
}
},
Referencing: https://github.com/powmedia/backbone-forms/blob/master/src/editors/select.js#L149
There's not really a good way to know if an ID should always be numeric or not, and "sometimes" returning an int vs string seems like a bad idea to me.
I am only suggesting this. More and more projects are moving to UUID's anyway which are strings. But even then it'd return a string if it was one. Im just wondering if an Int should be parsed more often than not it. I think in most circumstances a developer would be expecting an Int value if it can be parsed as one.. Not a big issue of course but this would save scattering parseInt through my forms
parseInt is very aggressive. It seems way better to subclass Select to do what you need than to make this the default behavior.
It's easy to make your own 'SelectInteger' editor:
Form.editors.SelectInteger = Form.editors.Select.extend({
getValue: function(){
var value = this.$el.val(),
parsed = parseInt( value );
return _.isNaN( parsed ) ? value : parsed;
}
});
declared in your model like any other editor:
var schema = {
"mySelect": {
"type": "SelectInteger"
}
}
Fair enough, I rolled my own editor as you suggested but I use it more than the default editor so thought that may be the same as the community.
Thanks
Dave Robertson Chief Technology Officer CricHQ E: [email protected] Ph: +64 (0)29 770 9595
On 15/11/2014, at 4:39 am, Ian [email protected] wrote:
parseInt is very aggressive. It seems way better to subclass Select to do what you need than to make this the default behavior.
It's easy to make your own 'SelectInteger' editor:
Form.editors.SelectInteger = Form.editors.Select.extend({ getValue: function(){ var value = this.$el.val(), parsed = parseInt( value ); return _.isNaN( parsed ) ? value : parsed; } });
declared in your model like any other editor:
var schema = { "mySelect": { "type": "SelectInteger" } }
— Reply to this email directly or view it on GitHub https://github.com/powmedia/backbone-forms/issues/428#issuecomment-63081595 .
It could be a common community requirement. I've never needed selects to produce ints, but I don't currently use a collection as options anywhere.
I see the argument for Selects whose options are a collection returning those values as an int, since it's explicitly using the id, something like -
getValue: function(){
var value = this.$el.val(),
parsed = parseInt( value ),
isCollection = this.schema.options instanceof Backbone.Collection;
return isCollection && !_.isNaN( parsed ) ? parsed : value;
}
but there's still the possibility that this could corrupt your values if the models in your collection had a custom idAttribute like "name" and values like "5 golden rings" or "3-restaurant.com"
To me, it seems much better as a separate class you explicitly use when you know you want ids.