I am on the process of changing jobs, so I would like to get an idea of what I do wrong in order to improve.
For this, I have created a small node module. It's very simple; it calculates linear motion (distance, velocity, time). I am interested in knowing what I do wrong, whether it's some crucial mistake or something secondary as the way I format my comments.
/**
* Checks whether two values are valid to be operated with
* @param {Float} operandA
* @param {Float} operandB
* @return {Boolean}
*/
const areValuesValid = (operandA, operandB) => {
if(isNaN(operandA) || isNaN(operandB)) return false;
if(operandA === null || operandB === null) return false;
return true;
}
/**
* Rounds a value to a max of two decimals
* @param {Float} val
* @return {Float}
*/
const round = (val) => {
return Math.round(val * 100) / 100;
}
/**
* Calculates the time in relation to the velocity and the distance
* @param {Float} velocity
* @param {Float} distance
* @return {Float}
*/
const calculateTime = (velocity, distance) => {
if(areValuesValid(velocity, distance) === false)
return 0;
if(parseFloat(velocity) === 0)
return 0;
return round(distance / velocity);
}
/**
* Calculates the velocity in relation to the time and the distance
* @param {Float} time
* @param {Float} distance
* @return {Float}
*/
const calculateVelocity = (time, distance) => {
if(areValuesValid(time, distance) === false)
return 0;
if(parseFloat(time) === 0)
return 0;
return round(distance / time);
}
/**
* Calculates the distance in relation to the velocity and the time
* @param {Float} velocity
* @param {Float} time
* @return {Float}
*/
const calculateDistance = (velocity, time) => {
if(areValuesValid(velocity, time) === false)
return 0;
return round(velocity * time);
}
module.exports = { areValuesValid, calculateTime, calculateVelocity, calculateDistance }
If anybody is willing to go the extra mile, I have created a repository with this code, which also includes some tests. Github. I would like to know if I go about the tests the right way, if the module structure makes sense and whatever else is criticizable.
1 Answer 1
Nice documentation. This code is clear and easy to read. Here are some basic nits:
You should probably follow standard JSDoc conventions. Don't do this:
/** * */
Do this instead:
/** * */
If you're nitpicky about the
@param
,Float
technically doesn't exist since all JavaScript numbers are floats. UseNumber
instead.See this SO post about rounding to 2 decimal places. Using it is completely up to you, there's nothing wrong with the way you're doing it. (This suggestion is slower, but looks cool I guess)
const round = val => { return +val.toFixed(2); };
You know the constraints of your
areValuesValid()
function, there's no need to=== false
.const calculateTime = (velocity, distance) => { if (areValuesValid(velocity, distance) && parseFloat(velocity) !== 0) { return round(distance / velocity); } return 0; }
Also, why the need to do
parseFloat(velocity)
? If you know all your arguments will be numbers, this is unnecessary. If you don't know the data type of your arguments, you should probably enforce that they are numbers usingtypeof
.
Happy coding!
-
\$\begingroup\$ Roger on the comments. I did
=== false
to avoid doing!areValuesValid()
I was told that evaluating to false requires (at the processor level) the inversion of the truth tables, so it takes a little (insignificant nowadays) more time. As for theparseFloat
, I was covering my ass in case somebody sends strings as params, which will work throughout the function except in that evaluation. I guess I could also remove theparseFloat
and evaluate with!=
instead of!==
. Thanks for the feedback \$\endgroup\$yBrodsky– yBrodsky2017年07月26日 23:29:58 +00:00Commented Jul 26, 2017 at 23:29 -
1\$\begingroup\$ It depends on what you want this to be used for. If this is a user facing module/api that other people will be calling, then strong argument type checks are good. If this is going to be used internally in a project of your own, you can establish reasonable expectations for the parameters and their types. Also, I'd appreciate if you accepted this answer if it helped :) \$\endgroup\$omgimanerd– omgimanerd2017年07月26日 23:49:55 +00:00Commented Jul 26, 2017 at 23:49
Explore related questions
See similar questions with these tags.