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
address@hidden
9, rue Alfred Kastler
BP 50752
44307 NANTES CEDEX 3
http://www.auguria.net
De: "Régis Houssin" <address@hidden>
À: "Posts about Dolibarr ERP & CRM development and coding" <address@hidden>
Cc: "Posts about Dolibarr ERP & CRM development and coding" <address@hidden>
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
Le 28 mai 2012 à 22:58, "Laurent Destailleur (eldy)" <
address@hidden> 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 :
Bof !!!
Le 25/05/2012 21:56, Régis Houssin a écrit :
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 |
address@hidden
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
address@hidden
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev
--
Eldy (Laurent Destailleur).
---------------------------------------------------------------
EMail: address@hidden
Web: http://www.destailleur.fr
Dolibarr (Project leader): http://www.dolibarr.org
To make a donation for Dolibarr project via Paypal: address@hidden
AWStats (Author) : http://awstats.sourceforge.net
To make a donation for AWStats project via Paypal: address@hidden
AWBot (Author) : http://awbot.sourceforge.net
CVSChangeLogBuilder (Author) : http://cvschangelogb.sourceforge.net
_______________________________________________
Dolibarr-dev mailing list
address@hidden
https://lists.nongnu.org/mailman/listinfo/dolibarr-dev