Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random network check #3021

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Random network check #3021

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6794d2
Random network check
zoic21 Jan 31, 2025
3228cbb
Reformulation
Hotfirenet Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions core/class/jeedom.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,13 +1078,6 @@ public static function cron5() {
}

public static function cron10() {
try {
network::cron10();
} catch (Exception $e) {
log::add('network', 'error', 'network::cron : ' . log::exception($e));
} catch (Error $e) {
log::add('network', 'error', 'network::cron : ' . log::exception($e));
}
try {
foreach ((update::listRepo()) as $name => $repo) {
$class = 'repo_' . $name;
Expand Down
1 change: 1 addition & 0 deletions docs/fr_FR/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
- Ajout d'un bouton de mise à jour sur la ligne "core" en plus de celui en haut à droite [LIEN](https://github.com/jeedom/core/pull/2974)
- Correction de l'écran "package" au niveau des packages python2 [LIEN](https://github.com/jeedom/core/pull/2973)
- Correction d'un bug lors de la mise à jour des équipements sur la page de batterie [LIEN](https://github.com/jeedom/core/pull/3008)
- La vérification de la connexion réseau est désormais effectuée de manière aléatoire toutes les 10 minutes afin d'éviter que toutes les box Jeedom ne tentent de se reconnecter simultanément en cas de microcoupure.

>**IMPORTANT**
>
Expand Down
19 changes: 19 additions & 0 deletions install/consistency.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,25 @@
$cron->setTimeout(10);
$cron->save();

$cron = cron::byClassAndFunction('network', 'cron10');
if (!is_object($cron)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je sais que le reste est fait de la même façon, mais ca serait surement plus précis de faire un if (!$cron instanceof cron). Qu'en penses-tu ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C’est une optimisation sans réel intérêt, car cron::byClassAndFunction('network', 'cron10') retourne déjà soit une instance de la classe cron, soit null si aucune instance n’est trouvée.

Dans ce cas, if (!is_object($cron)) et if (!$cron instanceof cron) sont redondants, car la méthode statique renvoie directement une instance de cron ou null. La vérification if (!$cron instanceof cron) n'apporte donc rien de plus que if (!is_object($cron)).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je tiens à préciser que ma suggestion ne porte pas sur l'optimisation mais sur la qualité et la maintenabilité du code.

L'utilisation de is_object() fonctionne techniquement, mais on pourrait communiquer plus clairement l'intention du code. Il y a plusieurs aspects à considérer :

  1. Typage explicite : instanceof cron ou === null documente directement dans le code le type attendu. Un nouveau développeur n'a pas besoin de consulter la documentation de byClassAndFunction() pour comprendre ce qui est retourné.

  2. Protection contre les changements : Si demain la méthode byClassAndFunction() est modifiée (cas très peu probable, mais possible) pour retourner autre chose qu'un objet cron (par exemple une stdClass contenant des données de cron), is_object() ne détectera pas ce changement qui pourrait introduire des bugs subtils.

  3. Contrat de méthode : Si la méthode retourne strictement cron|null, alors if (null === $cron) serait la solution la plus expressive. Elle indique clairement qu'on vérifie l'absence d'un cron pour le créer ensuite.

Ces pratiques s'alignent avec les principes SOLID, particulièrement le principe de substitution de Liskov, en rendant explicite les types attendus dans le code.

Copy link
Contributor

@Salvialf Salvialf Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bonjour,

Merci pour la leçon mais en l'occurrence c'est bien un objet qui est attendu ici.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un dernier point sur le sujet et je vous laisse tranquille :)

J'ai vu passer beaucoup de projets, je maintiens que la différence entre un code "qui fonctionne" et un code "professionnel" réside souvent dans ces petits détails de typage explicite et d'intentions claires.

Ce n'est pas une question d'optimisation ou de fonctionnement - ce code marche très bien. C'est une question de maintenabilité et de robustesse sur le long terme. Ces petites attentions font toute la différence quand on reprend le code 6 mois plus tard.

Bref, ce n'était qu'une suggestion basée sur mon expérience. Je sais que vous laisserez le code en l'état et ce n'est pas grave. Toutefois, un peu plus de professionalisme dans vos réponses seraient le bienvenue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je suis tout à fait d’accord avec toi. L’objectif est d’échanger et de trouver le meilleur code possible. Ces détails de typage et de clarté sont importants pour la maintenabilité et la robustesse sur le long terme.

L’échange reste ouvert, et l’essentiel est d’avancer ensemble vers une solution optimale.

echo "Create network::cron10\n";
$cron = new cron();
$cron->setClass('network');
$cron->setFunction('cron10');
$rand_min = rand(0,9);
$cron = '';
for($i=0;$i<6;$i++){
$cron .= ($rand_min+($i*10)).',';
}
$cron = trim($cron,',').' * * * *';
$cron->setSchedule($cron);
$cron->setEnable(1);
$cron->setDeamon(0);
$cron->setTimeout(60);
$cron->save();
}

if (!file_exists(__DIR__ . '/../plugins')) {
mkdir(__DIR__ . '/../plugins');
}
Expand Down