React: createContext(), hooks: useState(), useEffect()

Community feedback

• 

  Kostiantin•80

  @jebbaitedPosted 9 months ago

  If you don't mind I'll leave some comments to your code.

  1: Use Prettier to format code. It is much easier to read when formatted. Even with default settings.

  2: Use error-first approach if possible when writing conditions. Your code:

  function removeLastInput(){
    if(input.length){
      setInput(input.filter((el,i)=> input.length-1 !==i && el ))
    }
  }
  

  My code:

    function removeLastInput() {
      if (!input.length) return
  
      setInput(input.filter((el, i) => input.length - 1 !== i && el))
    }
  
    function replaceLastInput(value) {
      if (!input.length) return
  
      setInput(input.map((el, i) => (input.length - 1 !== i ? el : value)))
    }
  

  3: In switch/case construction you can use return instead of break. Less rows in general.

      if (e.target.value) {
        switch (value) {
          case 'RESET':
            return setInput([])
  
          case 'DEL':
            return removeLastInput()
  
          case '=':
            return calculateResult(concatNumbers())
  
          default:
            setCharToInput(value)
        }
      }
  

  4: Variable creation. You could create an array in another way. Your code:

  const firstArray = [...concatedNumbers]
  const secondArray = []
  .
  .
  .
  firstArray.forEach((el) => el && secondArray.push(el))
  

  My code:

  const firstArray = [...concatedNumbers]
  .
  .
  .
  const secondArray = firstArray.map((el) => el)
  

  Array.map() returns a new array and you don't need to push to another which you created above.

  5: Some conditions difficult to understand and maintain. Like this one:

   if (
          previousInput === value ||
          !previousInput ||
          previousInput === '.' ||
          isNaN(input[0] && isNaN(input[1])) ||
          (isNaN(previousInput) && isNaN(input[input.length - 2]) && input.length >= 2) ||
          (input.length === 1 && previousInput === '-')
        )
  

  Should be a good solution to create couple variables with good naming and then put them into this condition.

      const notNaN = isNaN(input[0] && isNaN(input[1]))
      const someName = isNaN(previousInput) && isNaN(input[input.length - 2]) && input.length >= 2
  
  if (
          previousInput === value ||
          !previousInput ||
          previousInput === '.' ||
          notNaN ||
          someName ||
          (input.length === 1 && previousInput === '-')
        )
  

  6: Try to not repeat similar components:

            <Button>7</Button>
            <Button>8</Button>
            <Button>9</Button>
            <Button className="del">DEL</Button>
            <Button>4</Button>
            <Button>5</Button>
  

  Create an array and then go throught it.

    const Buttons = [
      {
        name: '7',
      },
      {
        name: '8',
      },
      {
        name: '9',
      },
      {
        name: 'DEL',
        class: 'del',
      },
      {
        name: '4',
      },
      {
        name: '5',
      },
      {
        name: '6',
      },
      {
        name: '+',
      },
      {
        name: '1',
      },
      {
        name: '2',
      },
      {
        name: '3',
      },
      {
        name: '-',
      },
      {
        name: '.',
      },
      {
        name: '0',
      },
      {
        name: '/',
      },
      {
        name: 'x',
      },
      {
        name: 'RESET',
        class: 'reset',
      },
      {
        name: '=',
        class: 'equal',
      },
    ]
  
          <Keyboard onClick={handleClick}>
            {Buttons.map((button) => (
              <Button className={button.class ? button.class : ''}>{button.name}</Button>
            ))}
          </Keyboard>
  

  Also you don't need <Keyboard /> component. Just handle clicks on every button.

   <div className="kayboard">
            {Buttons.map((button, index) => (
              <Button className={button.class ? button.class : ''} onClick={handleClick} key={index}>
                {button.name}
              </Button>
            ))}
    </div>
  

  7: Let's change <Button/> component a little bit.

  export default function Button({ name, className, onClick }) {
    return (
      <button value={name} className={`button ${className}`} onClick={onClick}>
        {name}
      </button>
    )
  }
  

  Now you can use without children props. Cause why do you need it if you pass only button name.

    <Button
                className={button.class ? button.class : ''}
                onClick={handleClick}
                name={button.name}
                key={index}
      />
  

  8: ThreeStateToggle. Here we'll also avoid using same <input /> couple times. Create <Input /> component:

  const Input = forwardRef(function Input({ number, id, handleClick }, ref) {
    return (
      <>
        <span>{number}</span>
        <input
          ref={ref}
          onClick={handleClick}
          className="toggle"
          type="radio"
          name="toggle"
          id={id}
        />
      </>
    )
  })
  

  Then use it:

    const themeSwitchInputs = [
      { number: 1, id: 'dark', ref: dark },
      { number: 2, id: 'light', ref: light },
      { number: 3, id: 'violet', ref: violet },
    ]
  .
  .
  .
   <div className="tri-state-toggle">
          {themeSwitchInputs.map((input) => (
           <Input
              number={input.number}
              id={input.id}
              handleClick={handleClick}
              ref={input.ref}
              key={input.number}
            />
          ))}
  </div>
  

  About forwardRef read DOCS

  Marked as helpful

  1
• 

  Lukas•140

  @lukKoteckiPosted 9 months ago

  Than You very much for review. It's very helpful and enlightening.

  1. done

  2. fixed

  3. fixed

  4. leaved as it was because I'm not sure how to delete null indexes

  5. fixed - even me when first written was scared of these conditions

  6. fixed - I like this one

  7. fixed

  8. fixed -still not sure how it works but copy & paste helps

  Once again Thank You!

  0

  ---

  

  Kostiantin•80

  @jebbaitedPosted 9 months ago

  @lukKotecki 4.

  const secondArray = firstArray.filter((el) => !!el)
  or 
  const secondArray = firstArray.filter((el) => {
  if (el !== null) return el
  })
  or
  const secondArray = firstArray.filter((el) => el && el)
  

  You are welcome!

  Marked as helpful

  0