Project feedback JavaScript Calculator

Hello,

I finished the JavaScript Calculator project and am looking to get some feedback. Links:

It passes all the tests and I additionally added the option to use the calculator with the keyboard. Overall I am not as satisfied with my result, because I think that the code and overall structure of my app could be less complicated, but it does work as intended so I decided to go with it.

Also I really liked the design of the example project, which is why I decided to try and mimic it. Sometime in the future I am planning to go over this project again and that time take a different approach to solve this challenge and also use a different design that I like.

Edit:
I ended up doing some refactoring of my code today, because I wanted to shorten my code. I created a new function that holds the calculator logic and replaced the switch statement with if/else statements so I could use logical operators. This let me shorten my code for about a third (went from 300 lines to about 200 lines). If anyone is interested in changes, you can compare them from below links:

Hello @kpav,

I was reading your code and took me a while to find the calculator logic, I don’t think that the Button component is a good place for it.

Also, the use of magic numbers[0] make it difficult to understand:

...
else if (caseValue === "divide" || caseValue === "/") {
      setCalculations((prevState) => {
        let length = prevState.length;
        if (
          (prevState.split("")[length - 2] === "-" &&
            prevState.split("")[length - 5] === "/") ||
          (prevState.split("")[length - 2] === "-" &&
            prevState.split("")[length - 5] === "+") ||
          (prevState.split("")[length - 2] === "-" &&
            prevState.split("")[length - 5] === "*")
        ) {
          return prevState.slice(0, length - 6) + " / ";
        }
...
 ... 
    (prevState.split("")[length - 2] ... // why 2?
  ....
    return prevState.slice(0, length - 6) + " / "; // why 6? 

Cheers and happy coding :slight_smile:

Notes:

The term magic number or magic constant refers to the anti-pattern of using numbers directly in source code… The use of unnamed magic numbers in code obscures the developers’ intent in choosing that number, increases opportunities for subtle errors (e.g. is every digit correct in 3.14159265358979323846 and is this equal to 3.14159?) and makes it more difficult for the program to be adapted and extended in the future.

1 Like

Hello,

I am planning to add the Context API to the application and put the calculator logic there. That way it should be better since all of my business logic will be in the Context.

I added some comments to my code which hopefully help explain better what that code does. In a nutshell I wrote that to solve the User Story #13 after a lot of trial and error. User Story #13 says: If 2 or more operators are entered consecutively, the operation performed should be the last operator entered (excluding the negative ( - ) sign). The first if statement looks at what the last two operators in the calculation are (hence the use of length - 2 to find what character is the last, - 2 is used because there is also empty space present and - 5 checks what is the second to last character in the calculation).

In the second if statement that is used if there is no " - " operator present it suffices to check just the very last character, which is located at the length - 2 position (again I had to calculate in the fact that there is empty space present).

The last return statement uses - 6 to get rid of the last two operators (for example " + - " take all together 6 spots, because of the white space) and replaces them with whatever operator was clicked. Similarly in the second if statement that deals with the option where there is no " - " present it uses - 3, because only one operator needs to be replaced.

This logic is then repeated throughout my app for every operator to make sure it works everywhere as it should in accordance with the user story. I completely agree that this logic is difficult to understand, but it was the only way I managed to pass that user story.

Thank you very much for your feedback.

Hello @kpav,

I added some comments to my code

The length of each line of comments is too long, making it difficult to read (maybe is a good idea to use multi-line comments).

… In a nutshell I wrote that to solve the User Story #13 after a lot of trial and error.

I see, so your app evolved, it was not designed. This is really common, IMHO is because most people write code “at the function level”, they write functions not systems[0]. Another reason, is because they think using a programming language. The problem with that is a programming language is not the best tool to think[1]: it make difficult to express things that are often trivials with other tools, and the need to use a specific syntaxis can result in the waste of many hours fighting the language. Most of the time pen and paper is a better option. I really like to draw diagrams and tables for my apps, when I read code I have the tendency to try to “see” the logic of the programs, so is easier to use paper (maybe you can try to describe the logic of your app using other thing(not code, and it doesn’t have to be drawings)).

… I completely agree that this logic is difficult to understand

But why? I mean, the use of magic numbers is only a symptom.
Also, I really don’t have an idea of what data you are sending to calculatorLogic (function), explain what is the input and the output of a function is really important[2].

Cheers and happ coding :slight_smile:

Notes:

a regularly interacting or interdependent group of items forming a unified whole

  • [1] Leslie Lamport: Thinking Above the Code:
/**
     * Adds two numbers.
     * ... 
     * @param {number} augend The first number in an addition.
     * @param {number} addend The second number in an addition.
     * @returns {number} Returns the total.
     * @example
     *
     * _.add(6, 4);
     * // => 10
     */
    var add = createMathOperation(function(augend, addend) {
      return augend + addend;
    }, 0);
1 Like