Si vous aussi vous souhaitez refactoriser votre javascript vers du Clean Code, alors suivez le cours dédié au Clean Code de DevTheory : go.devtheory.fr/cours-clean-code?
Super, merci, très utile de rappeler ces concepts. Cependant, tous les tutos sur les flag parameters que je vois ont le même souci : on supprime les if-else if pour faire des fonctions, mais jamais n'est montré comment sont appelées ces fonctions (ici, les "sendXNotification"). Donc le périmètre couvert au final par ton code n'est tout à fait exact à celui couvert par la fonction d'origine. Hors, pour décider de quelle fonction appeler, il y aura bien à un moment donné un test d'un état...et donc probablement également un switch ou des if -else if... et ça n'est jamais montré.
Normalement tu n'envoies pas des notif sans savoir ce que tu fais en amont. Tu as toujours un context. Et si tu peux passer un type en paramètre, alors tu peux choisir la bonne function. const call = async () => { try { await request() sendSuccessNotification(...) } catch (error) { sendErrorNotification(...) // tu peux même passer l'erreur throw new Error(error) } } Et au pire, tu peux faire un wrapper qui aura pour uniquebut d'appeler la bonne fonction, grâce au type: const sendNotification = (type: 'error' | 'success' | 'info', text: string) => { const method = type === 'error' ? sendErrorNotification ? type === 'succes' ? sendSuccessNotification : sendInfoNotification // tu peux utiliser un mapper ici aussi return method(text) }
Normalement, on transforme pas les flags parameters en fonctions mais en classes. D'où le mot clé "type". Si c'est le type de quelque-chose , il faut en faire une classe. Quand on regarde les recommandations de Robert Martin (créateur du livre Clean Code), il faut éviter autant que possible les switches pour des objets à la place. Dans ce cas-ci, plutôt que d'extraire les différents types en plusieurs fonctions, créer une classe pour chaque type avec une méthode "send" dans chaque classe qui est responsable de sa propre implémentation. La fonction d'origine est alors réduire à un seul paramètre (l'objet de la classe) et aussi une seule instruction (appliquer le send de la classe).
Comme l'a dit Louis, il y a bien un if/else (ou autre comme try/catch) en amont, ce qui permet d'appeler la bonne fonction. Il est également possible de faire une fonction avec un niveau d'abstraction en plus afin d'y passer le type, ou alors une classe si tu développes plutôt en POO (je préfère rester sur du functional programming).
Excellente vidéo avec un code assez simple et une super explication qui permet de comprendre la logique et ainsi l'adapter à n'importe quel autre langage
Tu pourrais aussi - couvrir ta refacto grâce à un filtet de tests unitaires préalable - éliminer la duplication à trois endroits de « je crée un toast puis je le hide » - ne pas « export function » les fonctions qui restent privées à ton module: la mécanique de hide après délai - mettre des types plus précis que any - Éviter d’écrire des fonctions qui ont à la fois un effet de bord (afficher un toast) et retourne une donnée (la référence du toast). Mais ici ce n’est pas une fonction « publique » de ton module donc c’est moins grave.
Bof, j'aurais pas utilisé send que je réserverais à xhr/fetch. let toast = notification() ou new notification() toast.showSuccess(text).hide(2000) voire directement notification().showSuccess('text').hide(2000) toast.showInfo('text').hide(2000) Le chainage c'est la vie ;p Bon, on ne t'entends plus sur ta chaîne, qu'est-ce que tu ouilles ?
C'est possible oui, je préfère juste rester sur quelque chose de plus "functional programming" par rapport à de la POO ✅ C'est simplement ma préférence de codage
@@lmz-dev Je jardine bro. Et je m'occupe de mes enfants. Et je fais du sport. Et je tourne / monte une formation sur Angular. Et j'organise un concours de conférences :p
Moi j'aurais simplement gardé la même base en utilisant un switch/case et en laissant un trow en default pour le cas ou ton level n'est pas géré, pour le nom de la fonction je pense à toggleNotification, la verbosité c'est bien mais il ne faut pas exagérer … les fonctions sont assez courtes pour comprendre et les commentaires c'est pas pour les chiens :P Tu fais des setTimeout mais tu ne les clear pas (definitely a potential memory leak !), donc un petit callback en prévention serait pas mal ou alors même faire un retour de ce setTimeout. Pour le level on peut aller encore plus loin et déclarer un enum (comme c'est pas du TS, faire une constante d'un objet que tu exportes et restreindre ta signature à la valeur d'une property de cet enum, du coup plus besoin de ta default case). Merci pour la communauté en tout cas 😉Sans rancune mais pour moi passer de 19 lignes à 24 ce n'est pas vraiment du refactoring pour du code si simple.
Très cool ta vidéo! Tous les commentaires restent forcément à l'appréciation du développeur qui l'écrit, il y aura autant d'avis que de développeurs. Je rajoute ma couche : - je préfère lire : hideNotificationAfterDelay(notification, 5_SECONDS) plutôt qu'avec 5 * 1000, c'est beaucoup plus vite compréhensible. - Effectivement la fonction sendNotification n'informe pas qu'elle la cache ensuite, peut être qu'un truc du genre sendAutoHidingSuccessNotification(text, 5_SECONDS) aurait été plus clair. Encore une fois, aucun code n'est parfait, ton refacto est deja très bien. PS : respect parce que mettre son code public sur TH-cam avec la possibilité de se faire critiquer à chaque coin de rue, faut avoir les épaules
Merci beaucoup DevTheory pour cet exemple concret ! Ça me rassure dans mes démarches où on m'a déjà dit que j'étais trop "maniaque" alors qu'en fait non Par contre, tu appliques quoi comme règle en-dehors du SRP ?
Super la vidéo, sinon concernant les secondes ça aurait été préférable de créer une constante SECONDE qui évite au développeur qui lit le code deviner.
Merci ! C'est possible oui, après cela peut faire beaucoup de détails pour peu de valeur et donc ajouter des lignes de codes "inutiles", il faut aussi bien faire attention au ratio valeur/ligne pour ne pas tomber dans les abysses du clean code ahah
Très intéressant, mais c'est pas un refactoring, car ce n'est pas iso-fonctionnel, tu dois aussi changer ta manière d'appeler cette fonction. Tu aurais aussi pu ne pas exporter les fonctions internes :)
Je suis plutôt d'accord sur l'approche. D'abord on impose ce qu'on veut comme on le veut, puis on adapte les autres. Ici, il manque juste la 2eme partie, ou on adapte les appelants qu'on a impacté.
Hello ! Bon tuto mais pourquoi ne pas faire un objet, avec une option setDuration plutôt que hide duration After délai, avec la possibilité de faire notification.sendSuccess, sendError, sendInfo... ?
Très cool sur le principe, néanmoins le nom est très peu adapté pour le coup pour 2 raisons : - en quoi send insinue que il pourrait se cacher après s'être affiché ? En ce sens, send ou show ont le même "soucis" - send fait penser qu'on envoi le code quelque part (email, api, push...) alors qu'en fait on change l'UI ici, on n'envoie rien. C'est malheureusement le souci avec une grosse partie des vidéos "clean code", les exemples sont trop tiré par les cheveux. Il y a bien trop d'export (sans explications) et bien trop de fonctions sans expliquer que pour un sujet si petit ce n'était pas nécessaire, mais que pour une feature plus grosse c'était la bonne démarche. Je ne peux pas te blâmer vu que nommer les choses et 1 des 2 choses les plus dur en dev. Néanmoins, ça démarrait bien avec le verbe.
Si vous aussi vous souhaitez refactoriser votre javascript vers du Clean Code, alors suivez le cours dédié au Clean Code de DevTheory :
go.devtheory.fr/cours-clean-code?
Plus de vidéos de ce type stp (gratuit, je précise au cas où), au top
Tres bonne vidéo d'initiation au clean code 👏👏👏
Super, merci, très utile de rappeler ces concepts.
Cependant, tous les tutos sur les flag parameters que je vois ont le même souci : on supprime les if-else if pour faire des fonctions, mais jamais n'est montré comment sont appelées ces fonctions (ici, les "sendXNotification"). Donc le périmètre couvert au final par ton code n'est tout à fait exact à celui couvert par la fonction d'origine.
Hors, pour décider de quelle fonction appeler, il y aura bien à un moment donné un test d'un état...et donc probablement également un switch ou des if -else if... et ça n'est jamais montré.
Normalement tu n'envoies pas des notif sans savoir ce que tu fais en amont. Tu as toujours un context.
Et si tu peux passer un type en paramètre, alors tu peux choisir la bonne function.
const call = async () => {
try {
await request()
sendSuccessNotification(...)
} catch (error) {
sendErrorNotification(...) // tu peux même passer l'erreur
throw new Error(error)
}
}
Et au pire, tu peux faire un wrapper qui aura pour uniquebut d'appeler la bonne fonction, grâce au type:
const sendNotification = (type: 'error' | 'success' | 'info', text: string) => {
const method = type === 'error' ? sendErrorNotification ? type === 'succes' ? sendSuccessNotification : sendInfoNotification
// tu peux utiliser un mapper ici aussi
return method(text)
}
Normalement, on transforme pas les flags parameters en fonctions mais en classes. D'où le mot clé "type".
Si c'est le type de quelque-chose , il faut en faire une classe.
Quand on regarde les recommandations de Robert Martin (créateur du livre Clean Code), il faut éviter autant que possible les switches pour des objets à la place.
Dans ce cas-ci, plutôt que d'extraire les différents types en plusieurs fonctions, créer une classe pour chaque type avec une méthode "send" dans chaque classe qui est responsable de sa propre implémentation.
La fonction d'origine est alors réduire à un seul paramètre (l'objet de la classe) et aussi une seule instruction (appliquer le send de la classe).
@@arthurderyckere8732 Oui ça bien sur, j'aime beaucoup Strattegy dans le style.
Comme l'a dit Louis, il y a bien un if/else (ou autre comme try/catch) en amont, ce qui permet d'appeler la bonne fonction.
Il est également possible de faire une fonction avec un niveau d'abstraction en plus afin d'y passer le type, ou alors une classe si tu développes plutôt en POO (je préfère rester sur du functional programming).
Excellente vidéo avec un code assez simple et une super explication qui permet de comprendre la logique et ainsi l'adapter à n'importe quel autre langage
Merci beaucoup ! 🙏
Merci très instructif...
Merci pour cette vidéo très intéressante !
Tu pourrais aussi
- couvrir ta refacto grâce à un filtet de tests unitaires préalable
- éliminer la duplication à trois endroits de « je crée un toast puis je le hide »
- ne pas « export function » les fonctions qui restent privées à ton module: la mécanique de hide après délai
- mettre des types plus précis que any
- Éviter d’écrire des fonctions qui ont à la fois un effet de bord (afficher un toast) et retourne une donnée (la référence du toast). Mais ici ce n’est pas une fonction « publique » de ton module donc c’est moins grave.
1000 merci et un abonné de plus 👏👏👏👏👏👏
Merci !! 😃
Très bonne vidéo !
Sympa la vidéo, vraiment cool et intéressant pour tous les devs, expérimenté ou pas !
Génial
Superbe vidéo bro.
Bof, j'aurais pas utilisé send que je réserverais à xhr/fetch.
let toast = notification() ou new notification()
toast.showSuccess(text).hide(2000)
voire directement
notification().showSuccess('text').hide(2000)
toast.showInfo('text').hide(2000)
Le chainage c'est la vie ;p
Bon, on ne t'entends plus sur ta chaîne, qu'est-ce que tu ouilles ?
Merci ! 😁
C'est possible oui, je préfère juste rester sur quelque chose de plus "functional programming" par rapport à de la POO ✅
C'est simplement ma préférence de codage
@@lmz-dev Je jardine bro. Et je m'occupe de mes enfants. Et je fais du sport. Et je tourne / monte une formation sur Angular. Et j'organise un concours de conférences :p
Moi j'aurais simplement gardé la même base en utilisant un switch/case et en laissant un trow en default pour le cas ou ton level n'est pas géré, pour le nom de la fonction je pense à toggleNotification, la verbosité c'est bien mais il ne faut pas exagérer … les fonctions sont assez courtes pour comprendre et les commentaires c'est pas pour les chiens :P Tu fais des setTimeout mais tu ne les clear pas (definitely a potential memory leak !), donc un petit callback en prévention serait pas mal ou alors même faire un retour de ce setTimeout. Pour le level on peut aller encore plus loin et déclarer un enum (comme c'est pas du TS, faire une constante d'un objet que tu exportes et restreindre ta signature à la valeur d'une property de cet enum, du coup plus besoin de ta default case). Merci pour la communauté en tout cas 😉Sans rancune mais pour moi passer de 19 lignes à 24 ce n'est pas vraiment du refactoring pour du code si simple.
Super 👍
Au top, merci
Très cool ta vidéo!
Tous les commentaires restent forcément à l'appréciation du développeur qui l'écrit, il y aura autant d'avis que de développeurs.
Je rajoute ma couche :
- je préfère lire : hideNotificationAfterDelay(notification, 5_SECONDS) plutôt qu'avec 5 * 1000, c'est beaucoup plus vite compréhensible.
- Effectivement la fonction sendNotification n'informe pas qu'elle la cache ensuite, peut être qu'un truc du genre sendAutoHidingSuccessNotification(text, 5_SECONDS) aurait été plus clair.
Encore une fois, aucun code n'est parfait, ton refacto est deja très bien.
PS : respect parce que mettre son code public sur TH-cam avec la possibilité de se faire critiquer à chaque coin de rue, faut avoir les épaules
Merci pour ce retour détaillé très intéressant ! Et content que tu es apprécié la vidéo :)
les fonctions à responsabilité unique c'est la vie
encore plus clean si les “newNotification” seraient des const et pas des let
Merci beaucoup DevTheory pour cet exemple concret !
Ça me rassure dans mes démarches où on m'a déjà dit que j'étais trop "maniaque" alors qu'en fait non
Par contre, tu appliques quoi comme règle en-dehors du SRP ?
Super la vidéo, sinon concernant les secondes ça aurait été préférable de créer une constante SECONDE qui évite au développeur qui lit le code deviner.
Merci ! C'est possible oui, après cela peut faire beaucoup de détails pour peu de valeur et donc ajouter des lignes de codes "inutiles", il faut aussi bien faire attention au ratio valeur/ligne pour ne pas tomber dans les abysses du clean code ahah
Très belle vidéo. Ce serait possible d'avoir un follow-up plus long avec l'utilisation concrète du Toast et des explications supplémentaires ?
Merci ! Je vais voir ça :)
Très intéressant, mais c'est pas un refactoring, car ce n'est pas iso-fonctionnel, tu dois aussi changer ta manière d'appeler cette fonction. Tu aurais aussi pu ne pas exporter les fonctions internes :)
Ah oui, j'avais oublié ce détail ! Merci du rappelle ahah 💪
Càd iso-fonctionnel ?
@@z4k_39 C'est quand tu n'imposes pas au reste du code d'etre modifié.
Je suis plutôt d'accord sur l'approche. D'abord on impose ce qu'on veut comme on le veut, puis on adapte les autres. Ici, il manque juste la 2eme partie, ou on adapte les appelants qu'on a impacté.
Hello !
Bon tuto mais pourquoi ne pas faire un objet, avec une option setDuration plutôt que hide duration After délai, avec la possibilité de faire notification.sendSuccess, sendError, sendInfo... ?
Très cool sur le principe, néanmoins le nom est très peu adapté pour le coup pour 2 raisons :
- en quoi send insinue que il pourrait se cacher après s'être affiché ? En ce sens, send ou show ont le même "soucis"
- send fait penser qu'on envoi le code quelque part (email, api, push...) alors qu'en fait on change l'UI ici, on n'envoie rien.
C'est malheureusement le souci avec une grosse partie des vidéos "clean code", les exemples sont trop tiré par les cheveux.
Il y a bien trop d'export (sans explications) et bien trop de fonctions sans expliquer que pour un sujet si petit ce n'était pas nécessaire,
mais que pour une feature plus grosse c'était la bonne démarche.
Je ne peux pas te blâmer vu que nommer les choses et 1 des 2 choses les plus dur en dev.
Néanmoins, ça démarrait bien avec le verbe.
Merci pour ton retour ! Je suis plutôt d'accord avec toi sur les points que tu évoques ahah, dur d'être parfait mais merci pour ton avis ! 💪
Faire du clean code en js */pepelaugh/*
Une autre version pas clean mais clair et courte ... peut être
const gererNotifications = (type, text) => {
const notif = {
'success': { msg: 'Succès ! ', delai: 2000 },
'info': { msg: 'Information: ', delai: 5000 },
'error': { msg: 'Erreur ! ' }
}
if(notif[type]) {
let toast = Toast.show(notif[type].msg + text)
notif[type].delai && setTimeout(() => {
Toast.hide()
}, notif[type].delai)
}
}