*	[Online Feedback Management System for Education]_

Coding Best Practices

The goal is to make the code as if it was written by one person.

Code formatting

Revision control

Quality

Maintainability is our top priority. Less is more. Prefer simple over complex.

Style

Prefer standard way of doing something over creating special cases.
Try to be consistent with the existing code in naming, layout etc.
Follow coding style guides as much as possible. They are specified in the developer website.

Learning

We have some code to be used as “references” by new developers. When editing a file, make sure you have looked at the corresponding ‘reference’ file so that you understand the code you are editing. The reference files are:
UI:

Logic:

Storage:

CoursesDbTest.java, CoursesDb.java,

Spend some time going through the ResourcesBin to level-up your knowledge about all things related to what you are coding, such as tools, techniques, best practices, etc.
If you plan to become a core developer, do a horizontal slice.

Refining a slice

The purpose is to,

understand all layers or the system, including how testing is done at each layer, without having to learn the entire codebase.
calibrate your coding style and code quality to match the expected standard.

Choose an action assigned to you for the slice. e.g. instructorsCoursesPageAction

The corresponding slice includes the code that calls this action and called by this action.

e.g.

[ instructorCourses.jsp and related js/css/java files]→ [instructorsCoursesPageAction.java] → [ Some methods in Logic.java ] → [ Some methods in CoursesLogic.java ] → [Some methods in CoursesDb.java]

and the corresponding test code.

Your job is to study this slice, and improve it. The slice could already include some TODO items, but don’t limit yourselves to those items only. Try to improve code quality as well as test coverage. You can use the reference files mentioned above as a guide.

Remember to check test coverage using a tool such as eclEmma. This may be useful: Measuring server-side coverage of GAE applications using EclEmma

There is no need to create a separate issue for each slice, but do create a separate pull request. The branch name can be something like slice_instructorCoursesPage. When merging these pull requests, use the PR number instead of the issue number in the merge commit message. e.g. [4321] refining slice slice_instructorCoursesPage

Refactoring

Follow the boy scout rule (i.e. always leave the campground cleaner than you found it). Whenever you touch some code e.g., to fix a bug, try to do some clean up of that code as well. But do the extra refactoring as a separate issue. One issue should fix only one thing.

Comments

Use comments ONLY when the code is not-self explanatory. Avoid redundant comments. Even comment elements such as @return and @param can be omitted if they don’t add value.
Comments should be well-written. They should not be written as 'note to self' of the developer, but as explanations to help the reader.
Write header comments for all classes.
Write header comments for all non-trivial public methods.

User-visible text

Make sure the terminology used is understandable to users.

Code formatting

Use Eclipse to auto-format code, but select the area of code to be formatted first. Wholesale auto-formatting can create unnecessary merge conflicts.
After auto-formatting, you can tweak the layout to improve readability.
Try to keep code within the the traditional 80 char width. It is OK to exceed it slightly if it helps readability.
If you break a statement to multiple lines, have a look at the ‘line wrapping’ section of our Java coding standard.

Issue tracking

Everything you do for the project, even non-coding tasks, should be recorded in the issue tracker.
Use a descriptive issue name with a well-defined scope. Improve DevMan is not good because the scope is not clear (improve in what way?). Add issue name convention to the DevMan is a better name.
All ongoing work should be reflected in the tracker correctly. The issue tracker should clearly show which issues are being handled at the moment, which are ready for review, etc. Use status labels (s.*) appropriately.
Do not start working on an issue before getting it assigned to you.
Resist the urge to do more than one task under one issue/PR unless they are tightly coupled and one cannot be done without the other. Specifically, don’t do refactorings unrelated to the fix. Instead, create a new issue and do as a separate PR.
Reason: Occasionally we have to ‘undo’ fixes done previously e.g. when we discover latent regressions introduced by that fix. Therefore, we should be able to undo any specific fix by reverting the merge commit of the relevant branch. If one branch merge brings in multiple fixes, we can undo one without the other.
Small issues are better than big ones. Big issues take longer to finish and have a higher risk of merge conflicts.

Revision control

Go through your change set before pushing it. Ensure all changes are as intended.

Security

When coding the UI (e.g., JSP pages), any user-entered value (e.g., Course name) should be sanitized before displaying it back. This is to minimize cross-site scripting.

---End of Document---