search

heldentodd / xray-diffraction

This repository contains the code for one simulation, but eventually three are planned: bragg-law, Single Crystal Diffraction, and Powder Diffraction.
GNU General Public License v3.0
1 stars 1 forks source link

use assert #13

Closed pixelzoom closed 4 years ago

pixelzoom commented 4 years ago

Related to #1 (code review):

Assertions should be used appropriately and consistently. Type checking should not just be done in code comments. Use Array.isArray to type check an array.

I see only 3 occurrences of assert, and they all appear to be from the simula-rasa template that was probably used to create the skeleton for this sim. So I'm guessing that assertions are not part of your programming toolbox.

Assertions are used to catch programming errors by verifying assumptions, including things like validating the values provided to functions, or the results of a complicated computation. (They are not used to check user input. Use throw Error for problems that are caused by external input your program.)

PhET's implementation of assertions can be found in assert/assert.js. It's a preload, so you do not need to explicitly import it. Assertions are conditionally enabled. They are disabled by default, enabled by running with ?ea. And to improve performance, they are stripped out of "built" sims.

Assertions are used like this:

assert && assert( expression, message );

For example:

assert && assert( value > 0 && Utils.isInteger( value ),value must be a positive integer: ${value});

Note that you always need to do assert && assert, because assert will be undefined if assertions are disabled or stripped out. And while the message is optional, it's recommended to provide a descriptive message.

Unless you really want to, I don't think it's necessary to go back and add assertions to your existing code. But going forward, it's highly recommended to use them. Especially in a weakly-typed language like JavaScript, assertions are one of your best tools for writing robust code. Let me know if you'd like to discuss assertions more.

heldentodd commented 4 years ago

Thank you for this information. I intentionally left out assertions (and tandems) when I was coding because I didn't want to write code that was "half-right". Also, I had enough to figure out. I'm glad to hear that assert doesn't slow down the final running code.

I'll be adding them in to the X-ray diffraction code this weekend.

pixelzoom commented 4 years ago

tandems are an aspect of PhET that you can safely ignore for now. They are associated with a PhET sub-project called PhET-iO, which you can learn more about at https://phet-io.colorado.edu. The overview is https://phet-io.colorado.edu/io-features/#customize.

heldentodd commented 4 years ago

Added several asserts to check incoming parameters on function calls.