1

I have a javascript function that takes two parameters, 'key' and 'value'.

If only one parameter is given then the 'key' parameter is given a default value and the undefined 'value' parameter gets the 'key' parameter value.

function thingSet(key,value){
    if(typeof value === 'undefined'){
        value=key;
        key='_default';
    }
    //... use key and value
}

The code works but I feel abit uneasy for some reason.

Are there better ways to do this?

zaf
  • 22,776
  • 12
  • 65
  • 95
  • See this: http://stackoverflow.com/questions/148901/is-there-a-better-way-to-do-optional-function-parameters-in-javascript – Tim Mar 06 '12 at 11:28
  • 1
    @Tim I saw that but my case is slightly different with the parameter swapping... – zaf Mar 06 '12 at 11:30

6 Answers6

2

You can refactor it like this:

function thingSet (key, value) {
    key = key || '_default';
    value = value || key;
    //... use key and value
}

That's nice short-circuit evaluation allowing you to set default values easily there.

Sarfraz
  • 377,238
  • 77
  • 533
  • 578
  • Thats not the expected logic. If only one parameter is given (the value) then your 'key' is set to the value - not the default. – zaf Mar 06 '12 at 12:22
  • @zaf: If that's what you are looking for and you understand the logic, you can change it to however you like but using `||` is easy way of setting defaults, that's the point here :) – Sarfraz Mar 06 '12 at 12:24
  • You should edit the 're-factored' code and highlight your point. As it stands your function is different to the original one. – zaf Mar 06 '12 at 12:41
2

This is pretty standard and heavily used "overloading" mechanism in javascript. You'll find it all over the libraries like jQuery.

Like many dynamic language constructs, there is a gap between what compiler can check for you and what you have to keep as a convention, perhaps documenting it thouroughly.

The power comes at a price of robustness. If you use this kind of tricks, you have to make sure everybody understands the implied API and uses it accordingly.

Tomas Vana
  • 18,317
  • 9
  • 53
  • 64
1

You can set default values like this:

function thingSet(key,value){
    key = key || '_default';
    value = value || key;
    //... use key and value
}

At least, that is what I make of your function. The unease may be due to the fact that in your function key may be undefined too, in which case the assignment after checking the condition if(typeof value === 'undefined') still may result in an undefined value

You can check for existence of at least one parameter using arguments.length.

function thingSet(key,value){
    if (!arguments.length) {
       alert('please supply at least one parameter'); 
       return true;
    }
    key = key || '_default';
    value = value || key;
    //... use key and value
}
zaf
  • 22,776
  • 12
  • 65
  • 95
KooiInc
  • 119,216
  • 31
  • 141
  • 177
  • Thats not the expected logic. If only one parameter is given (the value) then your 'key' is set to the value - not the default. – zaf Mar 06 '12 at 12:21
0

Seems fine to me. The only thing I'd have done differently would be a more direct comparison on value:

if(value == undefined){
aaroncatlin
  • 3,203
  • 1
  • 16
  • 27
0

I normally do this with JSON

myfunction = function(args){
    args.key =  (typof(args.key) == "undefined")?args.key = "_default":args.key;
    args.value =  (typof(args.value) == "undefined")?args.key:args.value;
}

myfunction({key:"something",value:"something else"})

that way you know which variable you are passing to the function and don't have to assume anything from within the function.

Evert
  • 8,161
  • 1
  • 15
  • 17
0

It's hard to discuss design questions on a dummy example, but I'd prefer a function that always accepts one parameter, which can be a simple value, or a hash of values. Consider this slightly more realistic example:

function setName(opt) {
    if (typeof opt != "object") {
        var p = opt.split(" ");
        opt = { first: p[0], last: p[1] };
    }
    $.extend(this, opt);
}

This can be used as person.setName('John Doe') or person.setName({last:'Doe'})

georg
  • 211,518
  • 52
  • 313
  • 390