Opentaps Coding Standards

Why Coding Standards?

Hey, that used to work! What happened to it?

Good code is not just code that runs today, but code that is easy to maintain and easy for other people to understand and work with. Here are some guidelines we've come up with to help make opentaps better, and we hope they will help your project as well.

opentaps Guidelines

Java is Your Friend. Using these tools will make your code easier and faster to write with IDE's such as Eclipse or NetBeans and easier to maintain and re-factor:

• Base Entity Classes
• Java Wrapper for OFBiz Services
• Screen Widget Actions in Java
• Java Classes for Constants
• POJO Service Engine

Create Tests Always create some demo data which could be used to test your use case.

Do not use minilang. A not-too-small book could definitely be written about why you should not use minilang, but how about:

• It has a non-standard syntax.
• It encourages un-object-oriented coding.
• It offers limited logging support.
• It is slower.
• It doesn't support a lot of basic operations of a real programming language.
• There is no backward or forward compatibility.

Even if you enjoy coding in XML, other developers will have trouble maintaining or debugging your code. You can always use Java or another Java scripting language, including beanshell, Groovy, Jython, JRuby, etc.

Do not use the form widget. You can make a simple form very quickly, but it is almost impossible to make a nice looking or sophisticated form. You'd also find yourself writing code like:

<set field="showCredit" value="${bsh:(postedBalance.compareTo(java.math.BigDecimal.ZERO) >= 0 
 && org.ofbiz.accounting.util.UtilAccounting.isCreditAccount(glAccount)) || 
 (postedBalance.compareTo(java.math.BigDecimal.ZERO) < 0 && 
 org.ofbiz.accounting.util.UtilAccounting.isDebitAccount(glAccount))}" type="Boolean"/>

Use the opentaps form macro or just freemarker/HTML instead.

This also includes the menu widget, the tree widget, and other related XML widgets.

Keep your ofbiz screen widgets simple. Use it as a basic tool to set data for a template. Don't have complex minilang calls (entity-condition, entity-one), and don't use it to layout your screens in XML. Use FTL templates, which are more flexible and allow better control of the page, and Screen Widget Actions in Java.

Do not overuse the service engine. The service engine is a convenient way to map parameters to backend business logic and have actions such as storing and updating data occur inside of a transaction. In those cases, using the service engine is a good idea, though it is probably better to write the services as POJOs, so that they can also be reused in other Java frameworks. Avoid using the service engine for looking up data, in services such as getInventoryAvailableForFacility, etc. which are not really transactional services. If you are just looking up data, consider writing a Java method. It is much faster.

Do not overuse transactions. Unless you are storing data, add a use-transaction="false" to your services XML definition so that it does not wrap it in a transaction automatically.

Returning error vs. failure When writing services, use returnError only when it is a fatal error that should abort the entire process. Otherwise, use returnFailure so that an error message is logged and returned, but the rest of the process could continue.

Keep names consistent. I once saw a developer spend four days trying to fix a bug because the webapp controller URL request was "updatePostalAddress" but the back end service was "updatePartyPostalAddress". Unless it is impossible, use the same field name on the entities in your services and web forms. Use the same webapp controller name as your service or servlet method. Use the name for your controller's request and view maps and the names for the screen, the freemarker template, and the beanshell script.

Organize your code logically. Put all code and services in the same application component. If the same service is used by different applications, put it into the commons component. Use the same sub-directory prefixes for all the freemarker, beanshell, script, services files.

Put in log messages whenever there is unexpected behavior. For example, this is very mysterious:

if (! "SALES_INVOICE".equals(invoice.get("invoiceTypeId")))
  return ServiceUtil.returnSuccess();

My service could run, and I might never know why. Instead, do this:

if (InvoiceTypeConstants.SALES_INVOICE.equals(invoice.getInvoiceTypeId())) {
    Debug.logInfo("Invoice [" + invoiceId + "] is not a sales invoice and will not be billed to a third party", module);
    return ServiceUtil.returnSuccess();
}

Comment your code! We recommend a least one line of comment per 10 lines of code. But good comments also need to explain why things are done the way they are done. Good comments don't just tell you what is happening but why. For example, would a comment like this help anybody?

     // set X to Y
     double x = y;

Highly unlikely: you just re-wrote your code in English. But this would be helpful:

     //  x and y coordinates should be the same when we re-calibrate.
     double x = y

Never assume your data is clean. If you ever write

creditCard = payment.getRelatedOne("PaymentMethod").getRelatedOne("CreditCard");  

you are guaranteed to get a null pointer exception sooner or later. Always check for null values first!