wiki:CODE_TASKS_REQUIREMENTS

Version 2 (modified by Tanya, 17 years ago) (diff)

--

Both implementor and reviewer must be very careful. Please check that you understand all of the above.

For each code task (no matter whether it is a library, external feature, bug fix...), should pass the following check list:

  • code
    • The code style is correct to our convention.
    • The code is easily readable.
    • There are no fake JavaDocs (JavaDocs without useful information).
    • The java-doc is complete enough.
      • For each element, the JavaDoc should describe clearly what is it (if this is not easy, then probably the design is bad)
    • It causes 0 errors.
    • It causes 0 warnings.
    • The @SuppressWarnings annotation is used only if really needed.
      • I know 3 cases for it -
        • unused, when something is used only by reflection
        • synthetic-access, when you want to touch something internal with inner class (although in many cases this may be avoided)
        • unchecked, when you are doing something complex with generic (note that this is also avoidable in many cases)
    • No inaccurate, or unclear XXX and TODO tags
    • It should not break other code (ensured by their tests).
    • other suggestions?
  • design
    • It is documented
      • (not only in the code).
    • It is simple and understandable.
    • It is easy to use.
      • (using it requires less work / small code)
    • It is error proof (hard to use it in a wrong way)
      • The compiler prevents most errors.
        • (it is better to have an abstract getKind() instead of forcing implementors to call setKind at construction)
        • (final may also help)
      • Most of other errors cause an exception
        • (defensive programming helps here)
    • When a library is used, it is used correctly.
    • There are no representation exposures.
    • There are no GOD (very long) methods.
    • There are no GOD (very long) classes.
    • The coupling is low.
    • No magic numbers (and strings)
    • No array members.
      • (unless there is really a reason)
    • No public instance fields.
    • No public non final fields
    • It has high cohesion.
    • (see http://en.wikipedia.org/wiki/Cohesion_(computer_science) )
    • It has low coupling.
    • It is possible to state in one sentence what every class or member is/does.
    • Classes and members are named correctly.
    • Every class (or attribute) is self responsible and self complete.
    • No code duplication (copy paste).
    • It is easily testable.
      • (when not, testing helpers are provided to make it such).
    • No other kind of smells.
  • It has good package structure.
  • It is logging correctly.
  • It has adequate error handling.
  • No premature optimization.
  • No premature abstraction.
  • tests
    • It has automatic use case tests, before implementing.
      • (they are very good to demonstrate the design for a library)
    • If it has demos, the demos are actually tests with main method.
      • (demos should be done at design time!)
    • It has enough unit tests to ensure its correctness
      • (listed at design time, passing at implementation time)
    • It has enough integration tests
      • (listed at design time, passing at implementation time)
    • It has enough system tests
      • (listed at design time, passing at implementation time)
    • It has written scenarios for manual testing
      • (when it is an external feature or a bug)
    • The tests had code coverage
      • (good code coverage does not mean good tests, but bad code coverage means bad tests)
  • other:
    • Enough information should be provided in the backlog.