add poc - #55
Conversation
CinquinAndy
left a comment
CinquinAndy
left a comment
There was a problem hiding this comment.
Globalement c'est plutôt nice !
Jsuis fière de toi !
Il te manque pas mal de petit détail comme des verifications de undefined / null etc etc...
T'as pas du tout de commentaire ! (pas bien ça!!!)
prend l'habitude -> 1 fonction -> 1 commentaire
1 truc avec un peu de logique -> 1 commentaire
c'est tout con mais ça fais la diff en terme de relecture pour toi et les autres !
| } from "@/app/(protected)/modules/mealworm/utils/methods/index.ts"; | ||
|
|
||
| const DataDisplay = () => { | ||
| const { rackList } = useRackListContext(); |
There was a problem hiding this comment.
si tu fais ça, ça veut dire que TOUT ce qui se trouve dans ton contexte "racklist" sera re render à chaque modification de cette dernière, est ce vraiment ce que tu veux ?
-> zustand pourrait t'éviter ça
-> ou juste une props passé en paramètre
There was a problem hiding this comment.
Je t'ai envoyé un message en mp sur discord 👍
|
|
||
| const DataDisplay = () => { | ||
| const { rackList } = useRackListContext(); | ||
| const selectedItem = useSelectedItemContext(); |
|
|
||
| let unifiedActionsList; | ||
|
|
||
| if (rackIsNotEmpty) { |
There was a problem hiding this comment.
const rackIsNotEmpty = formattedCrateActionsOfRack.length > 0;
en soit, tu n'as pas forcément besoin de stocker le résultat de ça, sauf si tu le réutilise à plusieurs endroit, en l'occurence, tu peux directement le faire dans ta condition, ça simplifierai un peu ton code je pense
There was a problem hiding this comment.
J'ai déclaré cette constante pour rendre le code plus compréhensible car je trouvais que l'expression formattedCrateActionsOfRack.length > 0 n'était pas très clair, en soit je ne commente pas ou très peu mon code car dans l'idéal le code doit parler de lui même. Pour le coup, je vais peut-être m'y mettre car dans ce cas-ci c'est plus pertinent!
| : [[]], | ||
| ) | ||
| .reduce((acc, curr) => [...acc, ...curr]) | ||
| .reduce((acc, curr) => [...acc, ...curr]); |
There was a problem hiding this comment.
un map sur un double reduce ? tu peux pas simplifié ça en vrai ? quel est l'intêret de faire ça ?
There was a problem hiding this comment.
tu pourrais essayer un truc du genre :
itemList = rackList
?.flatMap((rack) => rack.crate || [])
.flatMap((crate) => getFormattedCrateActions(crate));
(j'ai aps essayer, je sais pas si ça marche mdr)
There was a problem hiding this comment.
qu'est ce qu'il se passe si rack et crate sont undefined ? peut être un peux d'erreur handling pourrait être sympa
| name: crate.name, | ||
| action: action.actionType.name, | ||
| weight: action.weight, | ||
| created_at: new Date(action.created_at).toLocaleString(), |
There was a problem hiding this comment.
peut être que ça peut être plus safe de spécifié un format de date et de check ta valeur ici, potentiellement en fonction de la langue de l'os ou c'est déployer, ça pourrait péter
There was a problem hiding this comment.
( genre la date en fr ou en anglais est pas formatté de la même façon, donc potentiellement ça peut poser soucis )
| }); | ||
|
|
||
| export const getFormattedCrateActions = (crate) => { | ||
| if (!crate.action) { |
There was a problem hiding this comment.
c'est bien tu fais ta verif, pour savoir si crate est bien défini et tout, mais , pourquoi mettre un "?" juste après, tu fais une verif avant, normalement tu n'en a pas besoin
| }); | ||
|
|
||
| export const getFormattedCrateActions = (crate) => { | ||
| if (!crate.action) { |
There was a problem hiding this comment.
si tu veux check que c'est bien un array, tu pourrais utiliser Array.isArray pour une verif plus robuste ;)
| return crate?.action.map((action) => getFormattedAction({ crate, action })); | ||
| }; | ||
|
|
||
| export const getSelectedItemType = (selectedItem) => { |
There was a problem hiding this comment.
le retour de ce truc passé par un énum ça pourrait être mieux en terme de lisibilité
2 participants
No description provided.