Table of Contents
These code quality requirements are on top of our Implementation Pattern rules. If they contradict, the implementation pattern rules are the normative: https://docs.google.com/document/d/1zKO8gJ8rdPUUkj6xiti_E08Y4QKO2AB4F-qO5__Dtb0/edit?usp=sharing
If you find yourself copying the same code several times, extract that code into its own method Use language constructs like interfaces, traits to make code more expressive and reusable If there is a functionality already written by someone else in good quality, use it (if the project architect approves).
Naming is difficult, do it well. Be consistent in naming. With the rest of the project and yourself. Use Java naming standards for classes, variables and methods. Also in other languages, if there is no specific naming standard for the project. Choose descriptive and unambiguous names. Make meaningful distinction. Use pronounceable names. Use searchable names. Avoid encodings. Don't append prefixes or type information. Don't use abbreviations. The most specific the use of something, the longer its name. Do not be redundant. Do not use magic numbers. Use named constants.
Focus on API rather than patterns. First, write down the API for perfect scenario, observe how it feels, then jump to coding and make it work. If it helps understanding, extract expressions to local variables and/or methods. Tell, don't ask.
If you need to write comments to explain your code, it means you need to put it in a new method. The only place where comments are allowed is where the description is in the form of method comment. Do not comment out code. Remove it.
Exceptions of rules should always be thought out, explained and be available later. The right place to explain is right near the annotation (an exception from the "no comment" rule) If there are many exceptions, then either you do something wrong, or the rules should be changed.
If there is an if/else or switch/case, you might want to create specific classes for specific behaviour. If there are flag argument to a function, there should be more functions instead.
Separate concepts vertically. Declare variables close to their usage. Dependent functions should be close. Similar functions should be close. Place supporting functions below supported ones. Use white space to associate related things and disassociate weakly related.
Keep it simple, stupid. Extract the complex logic to other method. Avoid the use of else. Return early Throw exception One method should do one thing and one thing well Use short methods Use classes with only a few methods Keep number of fields low.
If you touched a code, you should leave it cleaner than you found it.
Do not mix data and behaviour in models: a class should be only one of them. Prefer data structures to behaviour. A code should only know its direct dependencies. Use dependency injection to break dependencies.
Treats are implemented using the related constructs of the given language. In multiple inheritance languages (like Python), a trait is a mixin. In purely procedural languages (like C), a trait is a visitor. In Java: A trait (e.g. Voteable) is an interface with default, static and private methods. The trait extends the interfaces providing the needed getters and setters (e.g. VoteInterface), and functionalities e.g. (IssuesBallots).
Do not mix data structure and behaviour in models: a class should be only one of them. An object has hidden data and exposed behavior. Data structures, have exposed data, and no behavior. Entities (e.g. VoteEntity) are dumb container to hold data, which might get persisted.
Objects (eg Vote) are visitors to corresponding entities (e.g. VoteEntity), and their sole purpose is to guard invariants. They use whatever constructs the language provides to guard internals. If no such thing in the language, a set of rules and static code analysis makes sure that protecion is there. In Java: They implement the constructors, getters and setters in a way that makes sure that invariants are adhered to. Objects have a corresponding interface (e.g. VoteInterface) defining only the getters and setters which are needed by business logic. Objects can signal the set of traits applicable to them by "implementing" them (while actual code is in the interface).
keep business logic in model layer
You are not allowed to write any production code unless it is to make a failing unit test pass.
You are not allowed to write any more of a unit test than is sufficient to fail; and compilation failures are failures.
adhere to clean code rules here as well separate setup, exercise, verify, and teardown optically do as much as you can do in setup and teardown methods, optimally leaving out them from the tests themselves complicated things should be organised out into test helper classes in end-to-end testing, think in processes and steps
The tests should organize actions on actual states. Do not wait a specific amount of time: - use callbacks instead of polling - if the only way is polling, set the poll frequency low, and time out after a reasonable number of attempts. If the test needs an interface for observing the actual state then create an interface for it. If the behaviour is time-dependent, then dependency inject the clock
Therefore there should be one assertion in one test.
The documentation is generated from test case descriptions; test case descriptions should fit into the documentation. The description of test cases should be kept accurate and with the role of them in the software documentation kept in mind. The description should explain the scenario and the expected result. If you take time to write the test description first, it is easier to write the test.
Test are annotated by whatever annotations the behaviour is supposed to have. The annotations are marked in the issue related to the behaviour. Annotations with the related information in the issue should be associated to every test with precisely copying the relevant strings: the annotations mark the relation between the architecture model and the code. You can use the annotation in relation to the test method or the whole class/file. If there are more than 3 tests testing the same combination of annotations, then they should be put to their own file, and the class/file should be marked. The way of marking is language dependent. E.g. with Java we use Java annotations, with C we use comments. Example annotations: @TestedFeature the high level feature to which the tested behaviour relates to @TestedOperation the operation of which we test a feature of @TestedComponent the application component (e.g. class) we are testing @TestedBehaviour the behaviour which we are testing
If it is not possible, the reasons should be clearly explained in the pull request, and the needed work should be clearly far beyond the scope and nature of the task at hand
module: one or more classes with a well-defined interface
Mocking is good. It shows interfaces. You don't have to wait for dependencies: mock them! When you mock a not yet implemented function/code path, create a test which: - test the mocked up function/code path - all of the test annotations contain the string "Unimplemented" - uses assertUnimplemented
you should include the issue number in the pull request there should be different pull requests for different issues it is okay to issue a pull request, fork a branch from it, continue your work, and issue a pull request in that. Just reference the second pull request from the first please rebase your code before issuing pull requests
modifying the configuration and scripts determining the working of the CI should only be done with clear indication of the reasons in the pull request, and should not result in less stringent quality checks
https://nvie.com/posts/a-successful-git-branching-model/ The code should be based in latest development branch, and the pull request should go into the same. Create feature branches for all your work items.
using the CI mechanisms for gaining unauthorized access, or for any other purpose than building and quality checking the project is strictly forbidden
While writing functionality someone else have already written is waste of effort, a dependency should be good quality and with appropriate licence. Introducing a new dependency should only be done with the approval of the project architect.