Query about the offbiz code

[Ambient Solution <so...@gmail.com>]

Hi ,



After seeking offbiz code , I found that there are lots of small work
required in making offbiz code as standard code. Like

1) There are so many hard cocded string at every place. Like  "user-login"
used as a string every place , Can we moved it to a constant file and refer
that string from constant file. this will be a best coding practice.
2) In  ConfigXMLReader.ControllerConfig

  There are some variable defined which are public.

*

public* *Map* configMap = FastMap.*newInstance*();

*public* *Map* handlerMap = FastMap.*newInstance*();

*public* *Map* requestMap = FastMap.*newInstance*();

*public* *Map* viewMap = FastMap.*newInstance*();

*public* String defaultRequest = *null*;
   I found some of these ,there meight be a possiblity offbiz have more
issue like that.

   Can we fix this type of issue. I know these are the small issue , but as
per the coding standard and practice wise  If we follow coding standard
define by Java , We will make offbiz as a great product , which will
fullfill all aspect of funcanality and technically.

  I am refereing to relese 4.0 . If these issue are fixed in any other
branch , Please let me know If I am not aware of that things.


-- 
Regards,
Sumit
Solution Architect
Pune
skypee: deepaka231
+91-9221182885

Re: Query about the offbiz code

[David E Jones <da...@hotwaxmedia.com>]

On May 6, 2009, at 8:21 PM, Ambient Solution wrote:

> After seeking offbiz code , I found that there are lots of small work
> required in making offbiz code as standard code. Like
>
> 1) There are so many hard cocded string at every place. Like  "user- 
> login"
> used as a string every place , Can we moved it to a constant file  
> and refer
> that string from constant file. this will be a best coding practice.

Who says this is a "best coding practice"?

I learned about that in school too, but fortunately my teacher was  
good enough to explain when it is a good idea, and when it is a BAD  
idea and makes code actually more difficult to write, to read, and to  
maintain.

The scenario you presented here is a case where the value is self- 
explanatory, so using a constant to represent it is an anti-pattern  
and not a best-practice at all.

The case where using constant fields is a best-practice is when the  
value is a "magic" value, ie it is not self-explanatory in any way,  
like the number "42" for example. If you put that in code the question  
is: what does that mean? That is when a constant field is helpful to  
clarify the code.

Unfortunately this is a common confusion for less experienced  
programmers (I'm not saying you're not experienced, I don't know, but  
it does sound like someone pushed you in the wrong direction or didn't  
adequate explain the right direction at some point). Early in the life  
of OFBiz there were a lot of these sorts of constants and they cause a  
lot of pain and problems, and have mostly been eliminated.

> 2) In  ConfigXMLReader.ControllerConfig
>
>  There are some variable defined which are public.
>
> *
>
> public* *Map* configMap = FastMap.*newInstance*();
>
> *public* *Map* handlerMap = FastMap.*newInstance*();
>
> *public* *Map* requestMap = FastMap.*newInstance*();
>
> *public* *Map* viewMap = FastMap.*newInstance*();
>
> *public* String defaultRequest = *null*;
>   I found some of these ,there meight be a possiblity offbiz have more
> issue like that.

Sometimes public fields are an issue, sometimes they aren't. In your  
personal opinion (textbook answers are no good), what sorts of  
problems will this cause? Why is bad? What aspect of maintenance or  
use would be improve by a different pattern?

>   Can we fix this type of issue. I know these are the small issue ,  
> but as
> per the coding standard and practice wise  If we follow coding  
> standard
> define by Java , We will make offbiz as a great product , which will
> fullfill all aspect of funcanality and technically.
>
>  I am refereing to relese 4.0 . If these issue are fixed in any other
> branch , Please let me know If I am not aware of that things.

These would never be fixed in a release branch as they are not bug  
fixes. In the trunk it is all a matter of identifying actual issues,  
and then getting the changes committed. For information on how to go  
about that see:

http://docs.ofbiz.org/display/OFBADMIN/OFBiz+Contributors+Best+Practices

-David