Wiki Spaces

Documentation
Projects
Resources

Get Help from Others

Q&A: Ask OpenMRS »
Discussion: OpenMRS Talk »
Real-Time: IRC Chat

Documentation

Skip to end of metadata
Go to start of metadata

OpenMRS tries to follow a common coding style in order to make our code base robust: more readable and more reliable (if the same code is created by three different developers, it should look and read very similar). Our style conventions are designed to promote consistency and good coding practices, while reducing common errors. If you feel that our coding styles could be improved (by changing, adding, or removing something), please bring your comments to the Developers Mailing List.

The following conventions are applied to code in the core code base, but we encourage module authors to adopt them as well.

 

File template

  • OpenMRS License Header. All code should start with the OpenMRS license header which can be found OpenMRS core repository.
  • File Length. Files should not exceed 2500 lines. If your class exceeds this length, consider refactoring your code to break it into more reasonably sized files. Overly large classes are harder to read.
  • One class per file. Do not put more than one class in your .java file(s).

Attribution

  • Attribution is to be done within commit comments according to the the comment conventions.
  • Attribution (either author or contributing authors) should not be placed into source code. When encountered, such attribution should be removed by (or with permission from) the original author(s). Contributions that require attribution (i.e., author(s) demanding attribution within the code contributions) will be graciously refused or removed from the repository.

 

(NOTE: this attribution-free coding policy took effect after version 1.1, so you may see some attribution in the existing code; we are gradually removing attribution from our code base)

 

 

Code Style

  • We recommend using the automatic formatting feature of an IDE (e.g., Eclipse) to generate consistently formatted code
    • In Eclipse, use Control+Shift+F (Command+Shift+F on Mac) to format a file.

    • OpenMRS formatting file for Eclipse: OpenMRSFormatter.xml
      • Install at Window -> Preferences -> Java -> Code Style -> Formatter
    • OpenMRS code template file for Eclipse: OpenMRSCodeTemplate.xml
      • Install at Window -> Preferences -> Java -> Code Style -> Code Templates
  • Include curly braces around if clauses, and loops even when there is a single statement inside, e.g instead of this

    It should be this:

  • For classes that extend BaseOpenmrsObject, try not to override the equals() and hashcode() methods UNLESS based on some specific semantics you have to do so
  • Avoid unnecessary else statements, normally you should be able to configure your IDE to catch this and warn you about it, e.g instead of this

    it should be this:

  • Use interfaces for return types that are collections that way the actual collection implementation used within a method can later be changed if need arises without breaking existing code.
  • When writing services, the @Transactional annotation should be added to the Implementing class at the class level and not the interface, then all methods that don't write to the database e.g getXXX methods should have their own Transactional annotation with the readOnly attribute set to true

Imports

Remove unused imports

Imports that are not used should be removed, remember that you introduce dependencies that are not needed!

Do not use wildcard imports

If you are importing multiple classes from a package, list them all individually, like in the following code block, since this makes it clear exactly what imported classes are in use in any file:

Do not do the following:

Please check your IDE settings it is often the IDE which merges several imports into a wildcard import.

Naming Conventions

  • Package names should be lowercase without punctuation, following Java specs.
  • Class and interface names should be capitalized (using CamelCase), begin with a letter, and contain only letters and numbers.
  • Method names should follow Java specs.
  • Parameter names should follow Java specs.

Strings

Concatenation

Use of StringBuffer in place of String Concatenation. Use the StringBuffer if you want to concatenate a number of string literals especially when this concatenation is not being done in a single line. e.g.
Don't write like String s = ""; s+= "Hey!"; s+'=" "; s+="Bye!" ;
Always write StringBuffer sb = new StringBuffer(); sb.append(Hey!); sb.append(" "); sb.append("Bye!"); String s = sb.toString();

Use of StringUtils

  • Do not do this: "if (s == null || s.equals("")) ...". Instead do "if (StringUtils.isNotEmpty(s) ..."
  • We have both the apache commons StringUtils and springframework StringUtils library. If possible, use the apache commons StringUtils method. However, if the spring StringUtils is already imported, use that one.

Do not comment out code

Do not leave commented out code in the source files. Just remove it. Others will think this has been left there for a purpose and thus will probably not dare to delete it which will leave this commented out code hanging around forever. There are some exceptions of course, such as: "When I'm pretty sure it needs re-enabled at some point, and to make sure I don't forget why that is, I leave a clear TODO comment, e.g. "TODO this needs re-enabled when TRUNK-XXXX is fixed". Most IDEs are good at parsing out all TODO statements." Rowan Seymour

More Conventions

  • System.out.println(). Avoid using System.out.println() in your code; rather, use the existing logging framework.
  • ex.printStackTrace(). Avoid using ex.printStackTrace() in your code; rather, use the existing logging framework to report stack traces.
  • Long method signatures. Method signatures should be kept under 400 characters in length.
  • Too many parameters. Method signatures should have less than 20 parameters.
  • new Boolean(). You should not instantiate the Boolean class; rather, use Boolean.TRUE and/or Boolean.FALSE.
  • foo = bar = 0. Avoid inline assignments; rather, break them into separate lines of code. Inline assignments can reduce code readability and may represent an error: equality (h1. ) accidentally entered as assignment (=).
  • Long lines. Lines should not exceed 125 characters.
  • Switch default case. All switch statements should have a default case if only to assert false, since future coding changes may introduce unexpected cases.
  • Switch fall through. All switch cases containing any code should end with a break or return statement to avoid accidentally falling through to the next case. In the rare case that you desire a fall through, the next case should be preceded with /* fall through */ to indicate your intention.
  • Magic Numbers. Literal numbers should only exist within constants, rather than being introduced directly in the code.
  • Literal Long Numbers. Always use a capital L for long numbers, since a lowercase L (l) is easily confused with a one (1).
  • equals() without hashCode(). Classes that override equals() should also override hashCode()'.
  • Nested If Statements. If statements should not be nested deeper than eight (8) levels.
  • Nested Try Statements. Try statements should not be nested deeper than three (3) levels.
  • No clone(). Classes should not override the clone() method.
  • Modifier order. Modifiers should follow the order suggested by the Java Language specification:
    1. public
    2. protected
    3. private
    4. abstract
    5. static
    6. final
    7. transient
    8. volatile
    9. synchronized
    10. native
    11. strictfp
  • (expression false). Do not test equality with true or false; rather, simplify the boolean expression by removing the comparison and, if necessary, adding a leading exclamation mark (!) if you are testing for false expressions.
  • if Statements. If statements should always be enclosed in braces. Always write "if (expr) {doSomething();}", never write "if (expr) doSomething();"
  • Loop Braces. There should be braces for the in the case of any loop even there is only one statement. Always write "while (condition) {doSomething();}", never write "while (condition) doSomething();"

Documentation

  • All classes and interfaces should, at a minimum, have a Javadoc comment at the class level

  • Public methods should have a Javadoc comment

  • Code that performs any non-obvious task should be documented for clarification

  • Specify the return value of every method. Return values that should be documented are: null values, empty collections (whether an empty sets or empty list is always returned, etc)

  • Test this return value assumption and annotate them with @should as described by Unit Tests

Exception Handling

General

  • catch (Exception e) {}. Do not catch Exception, Throwable, or RunTimeException; rather, you should catch more specific exceptions. Catching these high level exceptions can mask errors.
  • throw Throwable. Do not throw Throwable, Error, or RuntimeException; rather, you should throw more specific exceptions.

Creating Custom Exceptions

Create an intuitive hierarchy of exceptions within the package they belong, only creating new Exception classes for when the need to branch on different types of exception within code/webapp are needed (add as needed instead of making extra classes "just in case" for every possible variation).

For example:

org.openmrs.api.APIException
org.openmrs.api.PatientIdentifierException extends APIException
org.openmrs.api.MissingPatientIdentifierException extends PatientIdentifierException

and later add:

org.openmrs.api.DuplicatePatientIdentifierException extends PatientIdentiferException

when we realize the webapp needs to distinguish duplicates from invalid identifier exceptions.

Patching Third Party Libraries

New Public Methods and Classes

All new public methods and classes should have a @since annotation for the version when they were introduced. If the class is new, then it is enough to just put it at the class level instead of each method. Something like: @since 2.0

Deprecation

  • We deprecate methods that are part of our external public interface, instead of changing/deleting them in order to preserve backwards compatibility
    • External, public methods include:
      • service methods
      • public interface methods
      • domain object methods
    • Internal methods are not considered public, and therefore do not have to go through a deprecation cycle, and can be changed/deleted outright. These include:
      • DAO methods
  • We will delete all deprecated methods when we make a new version (eg from 1.x to 2.0)
  • Use both the @Deprecated annotation and the @deprecated javadoc comment
  • The @deprecated javadoc annotation should point to the new method that is replacing the current one
  • The @deprecated javadoc annotation should also mention the version when the deprecation happened. e.g: @deprecated As of 2.0, replaced by {@link #getEncounters(EncounterSearchCriteria)}

Security

Avoiding XSS scripting

  • In JSPs, use a core.OutTag  for every string type attribute (or even for every variable)
    • Example: instead of this:

    • write this:

  • When using our custom "openmrs:message" or tag, make sure to use htmlEscape unless you really  need html/js tags in the message:
  • StringEscapeUtils.escapeJavaScript() and StringEscapeUtils.escapeHtml() to escape any user-generated data in pages.
    • StringEscapeUtils is an Apache class and its java doc can be found here
  • In the reference application (with the UI framework), use ui.escapeJs(), ui.escapeHtml(), and ui.escapeAttribute()
  • Contextual Encoding is introduced for platform 2.0 and above.
    • Instead of StringEscapeUtils.escapeJavaScript()  use WebUtil.encodeForJavaScript()
    • Instead of StringEscapeUtils.escapeHtml() use WebUtil.escapeHTML() (This method signature is different from other signatures to preserve backward compatibility)
    • If the variable is used for an html attribute use WebUtil.encodeForHtmlAttribute()
    • You can find out the WebUtil class from here.
    • OWASP Encoder java doc can be found here.
       

 

4 Comments

  1. What are the conventions for naming of URLs and controllers?

  2. Is the UI framework still in use? I can't seem to find any references to it when searching the code base.

    1. Work on the Epic - Reference Application uses the UI Framework extensively. (But the existing web application that's under openmrs-core does not use it.)

  3. This post might explain why do we use <c:out value=${person.name}> instead of using directly ${person.name}

    http://stackoverflow.com/questions/291031/jsp-cout-tag

    Regards