Comparing potential energy of bond

[ddamelin]

Here's some feedback on the coding:

• There is a lot of repetition of code for each model. That makes it hard to maintain, because you need to make changes in multiple places every time a change is needed. Can you think of a way to reduce duplicate code?
• When a number is a constant that is used in several places, it is better to set that value to some variable (or parameter as may be necessary in the interactive), and use the variable. This is similar to the code maintenance issue described in the previous bullet.
• There are some code indenting conventions that are not being followed.

Sam may have some other suggestions.

[sfentress]

There is a lot of repetition of code for each model. That makes it hard to maintain, because you need to make changes in multiple places every time a change is needed. Can you think of a way to reduce duplicate code?

Honestly, unless a feature such as the ability to use scripts from external files that I suggested a few weeks ago gets implemented, there isn't really a practical way to "share" code between models.

The one way to do this would be to have a single model, instead of four different models. We have a number of models like this, for example "non-bonding" allows a user to change the elements, and it simply updates the atom properties.

It's hard for me to judge at this point whether it would be worth re-writing this interactive to use one single model.

[ddamelin]

Honestly, unless a feature such as the ability to use scripts from external files that I suggested a few weeks ago gets implemented, there isn't really a practical way to "share" code between models.

It looks like parameters are shared between models, so I was wondering if code to calculate might be tied to a model parameter. In each model there could be something like

    onPropertyChange('time',function(time){", .... or the onDrag() function currently being used

in the onLoad function. Then that function could update some parameter (ex. updatePEFlag) that has an "onChange" attribute which contains the code for calculating the PE.

@sfentress Do you think something like that would work?

[rishiloyola]

I did necessary changes in coding. You can now review it.

[ddamelin]

The behavior of the interactive seems pretty good now. I'm going to ask the Interactions team for feedback. However, I am noticing that the longer it runs the slower the dragging behavior is. (Or perhaps the more I drag the slower it gets.) I think this is because you have embedded a callEvery inside of an onDrag. Try removing the callEvery call. (This will also fix an indentation problem you have, as the callEvery function and its contents should be indented to show it is contained within the onDrag).

[ddamelin]

I made some additional whitespace changes. @rishiloyola please take a look for future interactives.