PR para corrección #34
Conversation
Resolviendo tateti
| }; | ||
|
|
||
| export default function MemoTest() { | ||
| const [colors, setColors] = useState(setGame()) |
There was a problem hiding this comment.
esto no está mal, pero creo que lo mencioné en la clase y está bueno tenerlo en cuenta a futuro:
si tenés una función para inicializar un estado (o una ref), en vez de ejecutar la función podés pasarle la función como argumento así
const [colors, setColors] = useState(setGame)
esto tiene la ventaja de que solo corre la función en el primer render del componente, así como está ahora se va a correr siempre. no significa que se vaya a actualizar mal, pero sí estás corriendo código que no es necesario, y si por ejemplo setGame fuera una función larga que hace muchas cosas, te podría llegar a ralentizar todo.
| function unselectColors() { | ||
| setTimeout(() => { | ||
| setColors(colors.map((color) => ( | ||
| { ...color, on: false } | ||
| ))) | ||
| setSelectedColors([]) | ||
| }, 200) | ||
| } | ||
|
|
||
| function discardColors(selectedColor) { | ||
| setTimeout(() => { | ||
| setColors((prevColors) => ( | ||
| prevColors.map((color) => ( | ||
| color.color === selectedColor ? | ||
| { ...color, discarded: true } : | ||
| color | ||
| )) | ||
| )) | ||
| setSelectedColors([]); | ||
| }, 200) | ||
| } |
There was a problem hiding this comment.
entiendo que hagas esto en funciones separadas para ponerles nombre, eso está bien. de lo que no estoy convencido es que el timeout lo estés seteando adentro, en vez de en el useEffect. Esto porque:
- es un poco dificil, leyendo el código de useEffect, darte cuenta que esto no corre instantáneamente
- se hace más difícil limpiar el timeout (cosa que no estás haciendo)
como regla, en un useEffect tenés que limpiar todas las cosas que queden "corriendo" en una función de return (como mostré en la clase, en la parte del title changer).
el useEffect completo me lo imagino así (sacando el setTimeout de estas dos funciones):
useEffect(() => {
if (selectedColors.length === 2) {
const timeoutId = setTimeout(() => {
selectedColors[0] === selectedColors[1] ?
discardColors(selectedColors[0]) :
unselectColors()
}, 200)
return () => {
clearTimeout(timeoutId)
}
}
}, [selectedColors, unselectColors])
| function DiscardedSquare() { | ||
| return ( | ||
| <div | ||
| className="memotest-square discarded"> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
este DiscardedSquare podría ser una variante del de arriba, pasándole una prop discarded. al ser similares y representar medio lo mismo me parece más limpio, pero no es crucial.
| const winner = getWinner(tiles, currentPlayer); | ||
| const gameEnded = winner[0] === true ? true : false; |
There was a problem hiding this comment.
se me hace un poco raro llamar a cada valor del array de getWinner sin nombre, sobre todo más abajo en el componente cuando hacés winner[1]. Queda más claro si hacés con desestructuración: const [gameEnded, winner] = getWinner(tiles, currentPlayer)
No description provided.