[Vtigercrm-developers] An Impassioned Plea from an VTiger Integrator

[Richie]

Hi DG! 
 
Your comments are well taken. These will be in place at the earliest. 
I know that this will be a quick fix but instructions have been sent across to have the code properly commented from now on for any future development/bug fixes. 
 
Richie




---- Dennis Grant<dgrant at accuratetechnologies.com> wrote ---- 

  Ladies and Gentlemen,

I am currently employed as the lead integrator/programmer for VTigerCRM
at a medium-sized manufacturing company. My job is to maintain the
VTigerCRM server, and to customize our installation of VTiger to match
our business needs.

Sometimes this involves getting other systems (like Microsoft Great
Plains) to play nicely with the CRM; sometimes it means changing VTiger
code to better adopt the CRM to our particular business needs.

Our current installation is a heavily-modified (and I do mean HEAVILY)
modified 4.3.2 installation. To be honest, I painted myself into a bit
of a corner with the way I implemented our code changes, such that
upgrading to newer versions is a formidable challenge. My intent is to,
soon, bring up a 5.0 installation and start backporting our changes into
it, but this time with patch management tools to better manage upgrading
in the future.

In a way, it's a bit like maintaining my own private fork of something
like the Linux kernel, and so I intend on adopting the best practices of
those who have gone before me.

Anyway, I have occasion to spend a LOT of time in VTiger code, and while
I have not yet seen the V5.0 code in any great detail, I have a pair of
impassioned pleas for all current VTiger developers (in any version)

1) For the love of all that is holy, PLEASE COMMENT YOUR CODE! Those of
us who have to pop into a module to make a few tweaks to how it
looks/operates are hamstrung by the lack of comments in VTiger code. We
wind up having to work everything out from scratch, and that makes a
difficult job even more difficult. The fact that VTIger code, in
general, uses good descriptive variable names helps out a lot, but
comments would go even further.

Please please PLEASE get into the habit of commenting your code, even if
you think the functionality is obvious - you'll REALLY help guys like me
out.

2) I see a lot of code that looks like this:

if ($_REQUEST['module'] == 'module_name1' || $_REQUEST['module'] ==
'module_name2') {

 if ($some_parameter !='') {

 Do some stuff;
 }
}

What is missing here is any sort of exception handling. Not only does
this code fail silently if the tested assumptions aren't true (which has
caused some unusual bugs) it also fails to communicate to integrators
what the allowed/expected parameters of this code block are.

EVERY SINGLE IF OR IF/THEN NEEDS AN ELSE TO CATCH EXCEPTIONS, without
exception (heh) Failing to do this opens the door to bizarre bugs and
much wasted time trying to track them down. It also makes an
integrator's job that much more difficult.

Compare to this example:

// Prepare the foo object for writing to the database

// We are only supposed to be called from the module_name1 or
module_name2 modules
if ($_REQUEST['module'] == 'module_name1' || $_REQUEST['module'] ==
'module_name2') {

 // We need this to be set 
 if ($some_parameter !='') {

 Do some stuff;
 }
 else {
 print_warning("Expected some_parameter to be set in
do_stuff.php");
 }
}
else { 
 print_warning("Called with unexpected value of REQUEST[module] :
".$REQUEST['module']." from do_stuff.php.");
} 

I don't want to get all programmer grammer nazi here, but I've been
ripping my hair out by the roots for the last few hours, and it's all
totally unnecessary. 

DG

_______________________________________________
Get started with creating presentations online - http://zohoshow.com?vt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.vtigercrm.com/pipermail/vtigercrm-developers/attachments/20060713/488cba6b/attachment-0003.html