Dynnamically creating and updating similar state with onee method in React

Question

I am trying to use a single handleChange method for checkbox and then update their respective states accordingly. When I am trying to create dynamic function call its breaking. Let me know what I am doing wrong here and can utilize the same method to update check status on checkbox.

Function which is failing -

const handleChange = (event) => {
    const target = event.target;
    const value = target.checked;
    const functionName = `set${target.name}`;
    // console.log(functionName) // functionName = setisItem1 or setisItem2
    functionName(value);
  };

My sandbbox code - https://codesandbox.io/s/tender-hooks-guecv?file=/src/App.js[enter link description here]1

this will help you https://stackoverflow.com/questions/5905492/dynamic-function-name-in-javascript/9947842#9947842 — Monzoor Tamal, Aug 08 '20 at 15:39

Julia · Accepted Answer · 2020-08-08T15:50:48.117

6

What your code is doing is the equivalent of "setisItem1"(), which doesn't work, because a string is not a function.

@yaya's answer will probably work (and that's what I initially tried), but eval can be unsafe.

That's why I'd write it like this:

const functionName = target.name === "isItem1" ? setisItem1 : setisItem2;

edited Aug 08 '20 at 15:50

answered Aug 08 '20 at 15:45

Julia

1,950
1
9
22

@Julia Thanks a lot for the answer, my only concern remaining here is i have around 12-15 checkboxes, which I believe is hard to manage this way. Let me know if u can provide some inputs on this. – Nesh Aug 08 '20 at 16:01
Closet I can think of is a switch case here ..but let me know if some how I can make this clever :) – Nesh Aug 08 '20 at 16:03
2

Oof, yeah... You could create a mapping of strings to functions, like `const functionName = {"isItem1": setIsItem1}[target.name]`, but I think it'd be best to refactor those functions into one. – Julia Aug 08 '20 at 16:03
2

@Nesh if you have alot of checkboxes, it's better to store them on a single state, i put in on the updated answer. – yaya Aug 08 '20 at 16:46

yaya · Answer 2 · 2020-08-09T11:36:40.383

2

You can change it to:

    const functionName = `set${target.name}`;
    eval(functionName)(value); // cause functionName is an string

Update: as @christian said in comment, it's a bad practice. So you can use this instead:

let functionName = `set${target.name}`;
let nameToFunction = (name, param) => (new Function(
     "return " + name + '(' + param + ')'
));
nameToFunction(functionName, 5)();

Update 2:

Both of the above solutions doesn't make sence in normal react development. using dynamic execution of code makes the code more complex and harder to read. so it's better to avoid it completely. In your example, you just have alot of checkboxes, so the best way to deal with it is to put them in just one state. like:

  const [isItem, setisItems] = React.useState({});
  const handleChange = (event) => {
    const target = event.target;
    const value = target.checked;
    setisItems({...isItem, [target.name] : value});
  };

This way you don't need the dynamic execution anymore.

See it on CodeSandbox

edited Aug 09 '20 at 11:36

answered Aug 08 '20 at 15:39

yaya

7,675
1
39
38

2

The VM cannot optimize `eval` calls. This would dramatically slow down the performance of `handleChange`, which is likely called frequently. – Lewis Aug 08 '20 at 15:55
1

@Christian that's first time i ever heard an argument about it that makes sence. i heard alot in past that : `it's dangerous` . i did update the answer. – yaya Aug 08 '20 at 16:12
2

No worries. I always thought people were just nagging about it too, but the VM literally *can't* know what the code inside `eval` does, so it has to compile it *every time* and can't make any assumptions about what's inside. If it's a one-off call, *maybe* you can get away with it, but if we're talking React or state logic (where we might call something several times a second, i.e. 60x for animations), we gotta avoid it completely. – Lewis Aug 08 '20 at 16:21
@Christian Agree. Actually in alot of projects performance doesn't matter that much (like hobby / school / learning / startup projects), But i agree that we better to follow best practices to avoid doing bad-practices in production. – yaya Aug 08 '20 at 16:28
@Christian i found a great question about it : https://stackoverflow.com/questions/197769/when-is-javascripts-eval-not-evil are you sure it's performance heavy? cause it doesn't seems to be as alot of people talked about it there. and also one time i used it on a production website (it was for executing user codes.). i used it alot on that project , but didn't noticed any performance issue. – yaya Aug 08 '20 at 17:03
1

Running user input through `eval` is a horrible idea for security reasons. Hard to imagine a worse case scenario for `eval` use really. But the answer for the post you shared mentions performance as the first issue - like I said, the VM cannot optimize an `eval` script, it will be a "cold start" every run so to speak, no optimization benefits. Trust me, I totally hear you, I personally don't care about "best principles" but more about what works best - I've never found myself using it willy nilly, there's usually a better solution and it comes with a lot of drawbacks. – Lewis Aug 08 '20 at 17:58
@Christian makes sense. honestly in recent years i did never use it as well (expect some cases that there was't any other easy alternative, like executing the user code dynamically on client.). but sometimes people ask questions like this, they say they want to execute a code dynamically. so i i'll ask them like : so use `eval`. because for beginners it's an easy solution (i was using it when i was beginner also.). but in total, i'll try to not suggest it in future, cause it doesn't result in a clean code and architecture, and it makes the code complex. thanks for your explanations. – yaya Aug 08 '20 at 19:05
1

@yaya Thx for the Update 2 :) +1 – Nesh Aug 08 '20 at 19:23

Dynnamically creating and updating similar state with onee method in React

2 Answers2