Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a piece of code that composes method name to call from a string parameter. I don't feel it's a good thing to do but I'm not sure what can go wrong with this.

Here is a simplified snippet of that code:

switchToFilter = function(filter){
    var filterMethod = 'switchTo' + filter;
    myObject[filterMethod]();
}
share|improve this question
add comment

5 Answers

up vote 7 down vote accepted

At run time, the worst thing that could probably happen is that the method doesn't exist, so you get some sort of run-time error saying ERROR: method: 'switchToFoo' could not be found in context blah blah blah...

I'm not really sure if this type of code could be vulnerable to injection attacks. Where does the value for filter come from?

A bigger problem might be that whatever development tools you're using won't be able to work with this. For example, if you want your IDE to search for all places where switchToFoo is called, it won't be possible to find the ones called by the snippet you show above. Or if you want to refactor switchToFoo to switchToFou, you have to change the method name, and then change the code that passes the value for filter. You will know all about this, but this sort of thing usually trips up developers who are new to the project.

share|improve this answer
    
You are right. I was missing the chain in call stack right here when I was debugging –  Mohsen Dec 18 '13 at 23:35
add comment

The biggest risk is trying to access something that doesn't exist within the array structure. In the worst case, that will crash your application. In the best case, there wouldn't be any perceived operation going on.

If you wrapped that logic with some guard checks to make sure you weren't accessing something that wasn't there, it would be a little bit more robust. You could also consider adding logging for the errant requests that still manage to come in. With log information, you could hunt the problems down after the fact.

share|improve this answer
add comment

What can go wrong with composing a method name out of a string?

If myObject[filterMethod]() doesn't exist, you could crash your application.

If your application is part of the safety controls for nuclear missile fire control, you could be potentially disabling critical safety controls.

If other software for the controls doesn't account for this crash, you could allow anyone to fire ze missiles.

If someone is having a bad day, they could decide to try to launch the missiles and be surprised they launch.

If they launch the missiles, you could start a nuclear war.

If a nuclear war starts, you could end civilization as we know it.


FOR THE LOVE OF THE CHILDREN CHECK THINGS EXISTS BEFORE ACCESSING THEM!

share|improve this answer
10  
Before they are engulfed by balls of fire, the type theorists will yell "I told you row-types had real life applications!!" –  jozefg Dec 18 '13 at 19:33
1  
@jozefg: They always have to get the last word in, don't they? hmph –  FrustratedWithFormsDesigner Dec 18 '13 at 19:48
    
Assuming they did myObject.containsKey(filterMethod), or something similar, though, what would be the drawbacks? –  mowwwalker Dec 18 '13 at 20:55
1  
@mowwwalker I guess, well, some people really, really dislike children. That might be a disadvantage in this case? –  enderland Dec 18 '13 at 20:56
    
Am I missing something? The question asks about the problem with composing method name but this answer talks about non-existing method. Would a missing method be a problem in javascript regardless how it is called as long as no existence check? –  Codism Dec 18 '13 at 22:01
show 1 more comment

It's a form of reflection, which has its uses in dynamically-built applications (like a GUI builder for example) or plugins, but should be a matter of last resort outside those contexts.

In addition to the excellent reasons stated in other answers, it's a sign that your class hierarchy probably needs refactoring. Doing so will most likely make it much easier to reason about all your code. For one refactored example:

switchToFilter = function(filterName){
    var filter = filters[filterName];
    filter.switch();
}

This allows you to use inheritance to avoid duplication in the switch functions, and most likely has a side effect of providing a convenient place to put other filter-specific behavior.

share|improve this answer
add comment
  • If filter is from an external source like a user you have a potential source of unintended errors, possibly a security issue depending on the contents of myobject.
  • The lookup occurs at runtime so you don't get the typical benefits of type checking (existence, call signature matches function signature, etc)

That being said I do use dispatch tables much like what you have in your example. They are useful in particular instances. The key is understanding what the costs (and risks) are and making sure they don't out weigh the benefits.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.