JavaScript program that calculates the sum and difference of a pair of numbers

Question

The goal of the following openprocessing.org JavaScript program is to calculate the sum and difference of a pair of numbers and print the 2 results if they are both positive. I want to do the calculations in a function and return 3 values from that function (Boolean, sum and difference). It works properly but I have a few concerns:

1. My use of undefined in declaring myTriple and calcResults. Is undefined really needed?

2. The variables myTriple and calcResults are declared in exactly the same way. Is it possible to avoid duplicating the declaration details?

function setup() {
 createCanvas(windowWidth, windowHeight);
 background('#004567');
 
 let myTriple = {
 isValid: undefined,
 sum: undefined,
 difference: undefined
 }
 
 noStroke(); fill('white'); textSize(30);
 
 myTriple = calculateSumDiff(55,33);
 
 if(myTriple.isValid === true) {
 text(myTriple.sum, 100,200);
 text(myTriple.difference, 100,300);
 }
 
}
function calculateSumDiff(operand1, operand2) {
 
 let calcResults = {
 isValid: undefined,
 sum: undefined,
 difference: undefined
 }
 
 calcResults.sum = operand1+operand2;
 calcResults.difference = operand1-operand2;
 
 if( calcResults.sum>0 && calcResults.difference>0 ) {
 calcResults.isValid = true;
 } else {
 calcResults.isValid = false;
 }
 
 return calcResults;
}

Answer 1

Use const instead of let when possible

Declaring a variable with let tells JavaScript you intend to reuse the variable. If you don't actually plan on doing so, using const allows the JIT compiler to produce more efficient code as well as expressing your intention that the variable shouldn't be changed after it's instantiated. (This prevents silly bugs such as a variable being changed unintentionally.)

Instantiate variables with their intended values

There's rarely a reason to declare a variable and then assign it a value on the next line. You can simply declare the variable and provide it a value at the same time.

So this:

let calcResults = {
 isValid: undefined,
 sum: undefined,
 difference: undefined
}
calcResults.sum = operand1+operand2;
calcResults.difference = operand1-operand2;

can be written as this:

let calcResults = {
 sum: operand1+operand2,
 difference: operand1-operand2
}

Furthermore...

Object fields don't need to be declared

In JavaScript, objects can generally be treated like dictionaries, meaning you don't need to manually declare their inner fields when you instantiate the object. You can simply assign a value to an object's field at any point in time.

Embrace truthiness

As a general rule, you don't need to explicitly check for equality to true. If your variable already contains a boolean value, you can simply pass it as-is to the if condition. Additionally, in JavaScript, every value is described as either "truthy" or "falsey", meaning that if you pass them into a context that expects a boolean value (such as an if condition), the runtime will automatically coerce it into a boolean value.

So this:

if (myTriple.isValid === true) ...

can be written as this:

if (myTriple.isValid) ...

When combined with well-named variables, this pattern also allows for much more readable logic. (Compare how "If the value of myTriple-dot-isValid is equal to true" sounds vs. "If myTriple is valid".)

Boolean assignments don't need ifs

When you're trying to assign a boolean value to a variable, using the if (condition) foo = true; else foo = false; pattern is redundant. You can simply assign the expression to the variable directly.

So this:

if (calcResults.sum>0 && calcResults.difference>0) {
 calcResults.isValid = true;
} else {
 calcResults.isValid = false;
}

can be written as this:

calcResults.isValid = calcResults.sum>0 && calcResults.difference>0;

Use JSDocs

JSDoc strings don't just provide documentation for the programmer. They also allow you to tell the IDE what types a function takes as arguments and returns. This lets you get type information about the functions and variables you're working with throughout your codebase without having to rely on tricks or, worse yet, blind faith.

Use consistent formatting

Many people have strong opinions about what style convention you should be following, but at the end of the day, the only real convention to follow is to be consistent with whatever style you are using. For instance, don't use leading spaces, blank lines, and semicolons in some places throughout your code but not others.

Clear and consistent formatting improves readability and comprehension, while sloppy and inconsistent formatting makes code hard to read and, in extreme cases, can hide bugs that would normally be easy to spot.

Putting it all together

With the above, this is what your code might look like:

function setup() {
 createCanvas(windowWidth, windowHeight);
 background('#004567');
 
 noStroke(); 
 fill('white'); 
 textSize(30);
 
 const myTriple = calculateSumDiff(55, 33);
 
 if (myTriple.isValid) {
 text(myTriple.sum, 100, 200);
 text(myTriple.difference, 100, 300);
 }
}
/**
 * Returns the sum and difference of two numbers passed to the function. The result
 * is considered valid if both the sum and the difference are greater than zero.
 * @param operand1 {number}
 * @param operand2 {number}
 * @returns {{sum: number, difference: number, isValid: boolean}}
 */
function calculateSumDiff(operand1, operand2) {
 const calcResults = {
 sum: operand1 + operand2,
 difference: operand1 - operand2
 };
 calcResults.isValid = calcResults.sum > 0 && calcResults.difference > 0;
 
 return calcResults;
}

---

Honorable mention: Temporary variables

This "improvement" is more subjective, but an alternative approach to creating the calcResults object would be to precalculate the sum and difference and store them into temporary variables. This allows you to instantiate the object all at once rather than delaying computing a field until afterward, which arguably would lead to a more easily readable function overall.

/**
 * Returns the sum and difference of two numbers passed to the function. The result
 * is considered valid if both the sum and the difference are greater than zero.
 * @param operand1 {number}
 * @param operand2 {number}
 * @returns {{sum: number, difference: number, isValid: boolean}}
 */
function calculateSumDiff(operand1, operand2) {
 const sum = operand1 + operand2;
 const difference = operand1 - operand2;
 return {
 sum,
 difference,
 isValid: sum > 0 && difference > 0,
 };
}

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Thank you for contributing. Might you have read the other answers? One of them mentioned a few of the same aspects, which isn't a bad thing- just something to be aware of.
• Will.Octagon.Gibson: +1 Thanks for this comprehensive review!
• TorbenPutkonen: While JavaScript does not require you to declare the fields of an object when it is being created, not doing so creates extremely hard to follow code. Declaring the fields in advance communicates intent. "The programmer intends this object to have these fields." Not doing this forces the reader to go through, usually hundreds of lines of code, to figure out what an object contains. A good programmer knows when to not use counterproductive features. This particular example creates a situation where you can have a triple with only two fields (highlighting how "triple" is not a good name).
• Abion47: @TorbenPutkonen There are two problems with declaring object fields with arbitrary values. First, that data can persist in the event of a bug which can lead to subtle and hard to track bugs, and second, many JavaScript IDEs use the variable's declaration to infer its type so declaring it with arbitrary data (such as undefined) can cause the inferred type to not match the intended type. JSDocs avoids both issues and also conveys intent, and the other solution is to use TypeScript.

Answer 2

Procedural decomposition is a useful tool to manage complexity, and separation of "business logic" from user interaction common.

I think the naming of procedures can be improved here:

setup() doesn't say what is set up - it is more than just the canvas.
And then it goes on calling calculateSumDiff() and conditionally displaying something; it does not provide for value entry.

To split a hair, calculateSumDiff() establishes validity, too.
(Difficulty in finding a closely fitting name may indicate a problem in decomposition.)

The task description is a bit thin: is the required difference operand1 - operand2, or shall it be non-negative? → Document your implementation choice!
(I notice this depends on whether the they in if they are both positive refers to the results (whic I now take to be more likely) or the numbers in the pair. (Which is how I read that initially, possibly because I would have used two sentences, or if the latter are both positive.)

You use 4 blanks for code indentation where 2 seems more common - fine with me, I recommend to stick to that until not feeling a beginner any longer.

• Robin: "To split a hair, calculateSumDiff() establishes validity, too." Not only this, but most people have considered negative numbers to be valid for several hundred years. If I pass 50, 75, I'd expect to get 125, -25, NOT a blank screen.
• TomG: setup (and draw) are fixed functions in Processing. Like main in C, called by the runtime/framework. Yes, one could make separate "sub-setup" functions and call them from setup, but anyone used to processing will know this pattern.

Answer 3

I'm not a JavaScript guy by any means, but I can apply what I know from related languages and let others shoot me down...

In setup(), I don't think we need the initial empty myTriple; we can wait until we have a value for it:

 let myTriple = calculateSumDiff(55,33);

Similarly, in calculateSumDiff():

 let calcResults = {
 sum: operand1 + operand2,
 difference: operand1 - operand2,
 isValid: sum > 0 && difference > 0
 }

I think that we no longer need the variable here, and can immediately return the object we created.

Also, note that this is overkill:

 if(myTriple.isValid === true) {

We only need to test that isValid is any true value - it needn't be identical to true:

 if (myTriple.isValid) {

(Tiny style choice - note the space after if so it looks less like a function call).

• Will.Octagon.Gibson: +1 All your suggestions worked properly when I implemented them in my Open Processing program except the statement that begins, let calcResults = {. It produces an error when trying to compute isValid, it can’t access the value of sum.
• Toby Speight: Thanks for the feedback (I don't have a suitable test environment). I'll admit that one was a bit of a punt - a real JavaScript person may be able to improve on that.
• Džuris: "I don't have a suitable test environment" if you're on a computer, press F12 and use the console.
• Toby Speight: Not sure what you mean @Džuris - F12 is mapped to "Stick" in my window manager. Consoles are Ctrl-Alt-F1 etc, but they run Bash.
• kevinSpaceyIsKeyserSöze: The person means the developer tools of the browser which normally open via F12. You can also open the developer tools via right-click -> inspect (on any website).

Answer 4

I want to build on @Abion47's answer, because it hits on a lot of key points but misses two crucial items.

1. Your program should not just print successes; it should also print descriptive error messages. Returning a blank screen if someone inputs "10, 30" makes them assume the application is broken, not that it's behaving as intended.
2. Returning a triple is based on your function name is counterintuitive. The function calculateSumDiff should return the sum and the difference of two numbers. If you want to later decide negative numbers are a no-go, you should do that in the business logic at that point in time. As another dev, if I pass 25, 50 into the function, I'd expect the answer to be {75, -25}, not {75, -25, invalid}. If I wanted to check if either of them is negative, I'd check that both the sum and difference are >0, I would never think to check if calcuateSumDiff came back as "invalid"

To solve these two problems, I'd suggest changing Abion's answer to this:

function setup() {
 createCanvas(windowWidth, windowHeight);
 background('#004567');
 noStroke(); 
 fill('white'); 
 textSize(30);
 // I assume eventually you'll be getting this input from a user, so I pulled it out of the function call
 const input1 = 55;
 const input2 = 33;
 const myDouble = calculateSumDiff(input1, input2);
 
 if (myDouble.sum > 0 && myDouble.difference > 0) {
 text(myDouble.sum, 100, 200);
 text(myDouble.difference, 100, 300);
 } else {
 text(`${input1}, ${input2} did not create a positive outcome for both the sum and the difference`, 100, 200);
 }
}
/**
 * Returns the sum and difference of two numbers passed to the function. 
 * @param operand1 {number}
 * @param operand2 {number}
 * @returns {{sum: number, difference: number}}
 */
function calculateSumDiff(operand1, operand2) {
 return calcResults = {
 sum: operand1 + operand2,
 difference: operand1 - operand2
 };
}

Answer 5

There are only two hard problems in computing: cache invalidation and getting programmers to pay attention to naming things.

Taking the time to figure out good names is the key to being a good programmer. The trick is that if you cannot figure out a good name, your code is probably violating some of the well known good practices. The naming problem is a red flag that tells you to fix the code before the problems get out of hand.

And programming is mostly about communicating with other people (sometimes this person is you a few years later). Your code should communicate to other people why it exists and why it does what it does. Naming is the first step in this communication. Comments and documentation come next.

A Triple should never exist in a computer program.

This link goes to a Java-related discussion that explains the problems with classes like Pair. It applies to "triple" too. In essence, the word "triple" does not communicate the reason why the object exists, it only communicates it's structure. And communicating structure is not worth a lot as it is already visible in the code. Should you need to add a fourth field into the object, the name "triple" no longer applies to it and you have a lot of unnecessary changes on your desk. A better name, which you already figured out, could be "calculationResult". Although that is a bit generic, it will do for of small piece of code like this. In a larger application you would need to describe the nature of the calculation in the name instead.

The "my"-prefix should never exist in a computer program.

I know it is very very widely used in tutorials but that is because the people who write the tutorials are always hard pressed to find even remotely good examples for their articles and figuring out descriptive names for completely hypothetical cases is pretty time consuming and they cop out. And because most teachers are not very good programmers, they don't have the knowledge to even warn about this anti-pattern. So, take this as a rule of thumb:

Whenever you are about to use the "my" prefix, stop and think about a better name that describes why the entity exists. Always.

The field isValid is redundant and a source for errors.

What if you change the value of sum or difference after assigning the validity? You would also have to update the isValid field. If you forget that, the object's state will be in error. It's better to leave the validity as a function that checks the sum and difference when needed. Some programming languages have the concept of immutability. If you can guarantee that the sum and difference cannot change, then you can store the validity status in the object. This can be useful if calculating the validity is time consuming. But in this example, probably just make it a function.