Bonjour, le PHP (entre autres) étant mon métier de tous les jours, je me permet de faire quelques remarques sur le code:
ce script semble être développé "à l'ancienne", un fichier de 1500 lignes avec très peu de fonctions, et quasi-intégralement procédural, c'est un peu lourd à digérer,
je vois beaucoup de display("[..] Foo bar... ça aurait peut être été judicieux de factoriser le code pour générer le [..],
tu itères bien souvent avec readdir(), je ne suis pas vraiment sûr que ce soit nécessaire, tu pourrais déduire les noms de fichiers que tu cherches dans la majeure partie des cas et simplement faire un $filename = sprintf('/proc/%d/status', $pid); if (file_exists($filename)) { $status = file_get_contents($filename); /* ... */ },
tu fais du shell_exec() sans utiliser escapeshellcmd() ni escapeshellarg(), alors le plus souvent c'est un string literal donc c'est pas grave, mais à garder dans un coin de la tête pour plus tard,
de manière générale le code est très redondant, il pourrait être factorisé,
tu dis que tu ne trouves pas les regex lisibles, et je suis bien souvent d'accord avec toi, mais je dirais surtout: ça dépend, et dans ton cas en utilisant des regex par endroit, tu aurais du code plus concis (et plutôt lisible je pense, car les regex dont tu aurais besoin serait relativement simples).
Bon sinon l'outil semble fonctionnellement très bien, par contre je trouve ça dangereux de hardcoder les limites acceptables en ce qui concerne, par exemple, le nombre de threads: en effet, un nombre de process/thread sera bien souvent affiné selon le type d'application (I/O-bound ou CPU-bound), selon le nombre de CPU dédiés sur la machine, selon la RAM disponible, etc... Donc j'ai peur que toutes les valeurs soient de facto fausses pour beaucoup d'environnements.
Mais, petite note positive, c'est cool de reprendre ce genre de petit projet, je suis sûr que ça peut servir à beaucoup de monde. Apache est trop souvent bien trop mal configuré, alors c'est une bonne chose, ça permet de sensibiliser les gens à la configuration.
Et puis quant bien même je fais des critiques (que j'espère être constructives) l'important c'est que tu te fasses plaisir à développer cet outil.
# Outil intéressant mais...
Posté par Christie Poutrelle (site web personnel) . En réponse au journal [PHP] Apache Check, première release. Évalué à 10. Dernière modification le 03 août 2021 à 09:47.
Bonjour, le PHP (entre autres) étant mon métier de tous les jours, je me permet de faire quelques remarques sur le code:
ce script semble être développé "à l'ancienne", un fichier de 1500 lignes avec très peu de fonctions, et quasi-intégralement procédural, c'est un peu lourd à digérer,
je vois beaucoup de
display("[..] Foo bar...ça aurait peut être été judicieux de factoriser le code pour générer le[..],tu itères bien souvent avec
readdir(), je ne suis pas vraiment sûr que ce soit nécessaire, tu pourrais déduire les noms de fichiers que tu cherches dans la majeure partie des cas et simplement faire un$filename = sprintf('/proc/%d/status', $pid); if (file_exists($filename)) { $status = file_get_contents($filename); /* ... */ },tu fais du
shell_exec()sans utiliserescapeshellcmd()niescapeshellarg(), alors le plus souvent c'est un string literal donc c'est pas grave, mais à garder dans un coin de la tête pour plus tard,de manière générale le code est très redondant, il pourrait être factorisé,
tu dis que tu ne trouves pas les regex lisibles, et je suis bien souvent d'accord avec toi, mais je dirais surtout: ça dépend, et dans ton cas en utilisant des regex par endroit, tu aurais du code plus concis (et plutôt lisible je pense, car les regex dont tu aurais besoin serait relativement simples).
Bon sinon l'outil semble fonctionnellement très bien, par contre je trouve ça dangereux de hardcoder les limites acceptables en ce qui concerne, par exemple, le nombre de threads: en effet, un nombre de process/thread sera bien souvent affiné selon le type d'application (I/O-bound ou CPU-bound), selon le nombre de CPU dédiés sur la machine, selon la RAM disponible, etc... Donc j'ai peur que toutes les valeurs soient de facto fausses pour beaucoup d'environnements.
Mais, petite note positive, c'est cool de reprendre ce genre de petit projet, je suis sûr que ça peut servir à beaucoup de monde. Apache est trop souvent bien trop mal configuré, alors c'est une bonne chose, ça permet de sensibiliser les gens à la configuration.
Et puis quant bien même je fais des critiques (que j'espère être constructives) l'important c'est que tu te fasses plaisir à développer cet outil.