Sorry for french language:
Bonjour à tous,
Je réouvre rapidement cette discussion - je comprend très bien le fond de la
réflexion, un core propre permet un développement plus modulaire et donc
favorise l'enrichissement des fonctionnalités par le biais de modules externes.
J'ai cependant quelques retours à faire à ce propos. Je viens de travailler sur
le module de composition (Auguria) que des collègues ne travaillant plus dans
l'entreprise ont développés il y a un certain nombre de version (sans doutes
entre dolibarr 2.6 et 2.9).
Contexte: une demande client pour intégrer le module en version 3.1.2.
Partie du core commentée: htdocs/product/stock/class/mouvementstock.class.php
lignes 209 à 214
// Composition module (this is an external module)
/* Removed. This code must be provided by module on trigger STOCK_MOVEMENT
if (! $error && $qty < 0 && $conf->global->MAIN_MODULE_COMPOSITION)
{
$error = $this->_createProductComposition($user, $fk_product, $entrepot_id,
$qty, $type, 0, $label); // pmp is not change for subproduct
}*/
lignes 289 à 326
/**
* Cree un mouvement en base pour toutes les compositions de produits
* @param label Label of stock movement
* @return int <0 if KO, 0 if OK
*/
/* This function is specific to a module. Should be inside the trigger of
module instead of core code.
function _createProductComposition($user, $fk_product, $entrepot_id, $qty,
$type, $price=0, $label='')
{
dol_syslog("MouvementStock::_createProductComposition $user->id, $fk_product,
$entrepot_id, $qty, $type, $price, $label");
$products_compo = array();
$sql = "SELECT fk_product_composition, qte, etat_stock";
$sql.= " FROM ".MAIN_DB_PREFIX."product_composition";
$sql.= " WHERE fk_product = $fk_product;";
$resql = $this->db->query($sql);
if ($resql)
{
while ($item = $this->db->fetch_object($resql))
{
if ($item->etat_stock != 0) array_push($products_compo,$item);
}
$this->db->free($resql);
}
else
{
dol_syslog("MouvementStock::_createProductComposition echec update
".$this->error, LOG_ERR);
return -1;
}
// Create movement for each subproduct
foreach($products_compo as $product)
{
$this->_create($user, $product->fk_product_composition, $entrepot_id,
($qty*$product->qte), $type, 0, $label);
}
return 0;
}*/
Premier point : je suis parfaitement d'accord, ce code n'a rien à faire là.
(preuve: le temps requis pour le trouver...)
Deuxième point (bémol): enfin, dans un souci de cohérence avec le reste du core
de Dolibarr. D'ailleurs, dans un souci de cohérence avec le reste du core, je
me demande si implémenter les fonction CRUD de base à l'objet MouvementStock
avant de 'casser' le fonctionnement des modules dépendant de cette classe
n'aurait pas été un effort plus cohérent.
C'est le fond de ma remarque. Dans ce cas de figure, on se retrouve avec un
module à remettre d'équerre... Sans appui. L'objet MouvementStock n'a
simplement pas d'attribut, ni de méthode pour qu'on puisse l'utiliser de
manière intègre. J'ai bien remarqué la tentative (louable) de définir quelques
attributs pour que la fonction run_trigger puisse avoir les params requis par
le biais du $object... (cf l.216 à l.229:)
if ($movestock && ! $error)
{
// Appel des triggers
include_once(DOL_DOCUMENT_ROOT . "/core/class/interfaces.class.php");
$interface=new Interfaces($this->db);
$this->product_id = $fk_product ;
$this->entrepot_id = $entrepot_id ;
$this->qty = $qty ;
$result=$interface->run_triggers('STOCK_MOVEMENT',$this,$user,$langs,$conf);
if ($result < 0) { $error++; $this->errors=$interface->errors; }
// Fin appel triggers
}
Rappelons que nous sommes dans la fonction _create de la class (l.56:)
function _create($user, $fk_product, $entrepot_id, $qty, $type , $price=0 ,
$label='' )
En l'occurence, la fonction qui était utilisée avait été intégré dans la classe
(en commentaire au début du mail). Elle rappelle _create autant de fois que
nécessaire; elle a donc besoin de tous les paramètres de celle-ci pour un appel
correct. Manquent: $type, $price et $label .
Il serait possible de réinstancier l'objet dans le run_trigger pour obtenir les
données manquante de l'objet - s'il disposait de son id. cf l.91:
$mvid = $this->db->last_insert_id(MAIN_DB_PREFIX."stock_mouvement");
Variable $mvid qui n'est jamais rappelée où que ce soit. Les objets de type
MouvementStock n'ont juste jamais d'id.
Synthèse :
- je suis pour un code clair et propre, découpé et structuré.
- la suppression du code 'parasite' est une chose, mais elle implique que les
moyens de greffer les fonctionnalitées de ce code par les méthodes
conventionnelles soit misent en place.
- si le but est de forcer les développeur du module à reprendre le core pour
pouvoir appeller des méthodes standards, je trouve cela profondément
contre-productif. Cela fausse les chiffrages lorsque les développement sont des
réponses à des besoins clients, et je ne pense pas avoir besoin d'exprimer les
conséquence d'un tel état de fait.
J'ai donc quelques propositions:
1 - Éventuellement, laisser le code parasite s'il ne peut être externalisé par
la faute du core en lui-même. C'est la méthode 'sale', qui me parait être la
pire des propositions.
2 - Commenter le code parasite, mais étudier son fonctionnement - surtout
lorsque la fonction appelé est elle-même sur la même page. C'est la plus lourde
en temps pour le 'commentateur'.
3 - Commenter le code parasite, et envoyer un mail - sur la mailing list ou
directement au contact connu ayant développé le module.
La troisième possibilité me parait être un moindre mal. Peu coûteux en temps et
en effort. Peut-être qu'un projet libre implique que le code des autres est le
code de personne et
de tous...
En attendant, lorsque le code en question est vendu par une
entreprise, le responsable de la maintenance de ce code, c'est cette
entreprise. En ce sens, le commentaire sauvage, c'est du silent-patching.
Et à mon sens, c'est plutôt décourageant en terme d'investissement
professionnel (désagréable de chiffrer une matinée pour une modif qui finit par
prendre 3 jours, exemple arbitraire).
Cordialement,
AUGURIA
Anthony Poiret
[email protected]
9, rue Alfred Kastler
BP 50752
44307 NANTES CEDEX 3
http://www.auguria.net
----- Mail original -----
De: "Régis Houssin" <[email protected]>
À: "Posts about Dolibarr ERP & CRM development and coding"
<[email protected]>
Cc: "Posts about Dolibarr ERP & CRM development and coding"
<[email protected]>
Envoyé: Lundi 28 Mai 2012 23:10:22
Objet: Re: [Dolibarr-dev] Règle N°1
Sorry for french language:
Je suis d'accord pour commenter,
Par contre si des modules externes utilisent des fonctionnalités communes avec
un fort potentiel d'amélioration de dolibarr, il me semble plus judicieux de
les intégrer dans le cœur et pourront servir le cœur par la suite en
transposant ce qui a été fait sur les modules externes.
-----------------------------------------
Régis Houssin
Tél. +33633020797
http://www.dolibarr.fr
http://www.dolibox.fr
Le 28 mai 2012 à 22:58, "Laurent Destailleur (eldy)" < [email protected] > a
écrit :
The rule should be the opossite.
Every code that is not required must not exists. If some code is here in
advance for a future feature, it's the job of the developer adding such feature
to add comment to justify why useless code is added to prevent it to be removed
and to explain to quality guys (guys that tracks dead code) why such a code and
why it must be kept.
When a code is useless it can be commented if we are not sure it is used by
dolibarr but if we are sure it is not (no reference to a function for example),
there is no reason to be kept.
And no, the code of other is not the code of other: The code of other is the
code of nobody or everybody (We are an opensource project).
Le 25/05/2012 22:00, Cyrille de Lambert a écrit :
<blockquote>
Bof !!!
Le 25/05/2012 21:56, Régis Houssin a écrit :
<blockquote>
Règle N°1 : le code des autres c'est le code des autres ! ;-)
j'aimerais bien qu'on demande et/ou qu'on mette en commentaire le code
au lieu de le supprimer. Car si un code est présent c'est qu'il devait
servir.
Je me retrouve avec un module bancale et je n'arrive pas à retrouver le
pourquoi du comment...
Bon week end ;-)
Cordialement,
--
<mime-attachment.jpg>
Cyrille de Lambert
[email protected]
26 bis rue des olivettes
44300 NANTES Tél : +33 (0) 2 51 13 50 12
Mobile :+33 (0) 6 29 41 81 22
Fax : +33 (0) 2 51 13 52 88
http://www.auguria.net
_______________________________________________
Dolibarr-dev mailing list [email protected]
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
</blockquote>
--
Eldy (Laurent Destailleur).
---------------------------------------------------------------
EMail: [email protected] Web: http://www.destailleur.fr Dolibarr (Project
leader): http://www.dolibarr.org To make a donation for Dolibarr project via
Paypal: [email protected] AWStats (Author) :
http://awstats.sourceforge.net To make a donation for AWStats project via
Paypal: [email protected] AWBot (Author) : http://awbot.sourceforge.net
CVSChangeLogBuilder (Author) : http://cvschangelogb.sourceforge.net
</blockquote>
<blockquote>
_______________________________________________
Dolibarr-dev mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
</blockquote>
_______________________________________________
Dolibarr-dev mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
_______________________________________________
Dolibarr-dev mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
