@jebbaited
Posted
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