Guidelines for Smack Developers and Contributors
If you are planning on writing patches for Smack to add new features, improve existing features or fix bugs these guidelines should help you create patches that can be reviewed by the community and then be applied to the source repository.
This document covers guidelines about the code style, some code rules you should be aware of, how to test your code and how to create and publish a patch.
Besides the guidelines mentioned here, we recommend to read the excellent Kernel Guide to submit patches, which can be found at https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
Always remember the golden rule: Don't get discouraged. Re-submit!
Make yourself familiar with Smack. Read and explore documentation/. Also generate the current javadoc for the Smack version you are working with by calling gradle javadocAll.
If you use eclipse, then gradle eclipse will create the project files for every Smack (sub)project and fetch the required dependencies. Just import Smack and all nested subprojects into eclipse after gradle eclipse did its job.
Alternatively, if you use IntelliJ, you can use gradle idea to create an IntelliJ project that can be easily opened with the IDE.
Many developers use an IDE like Eclipse which comes with some handy tools to format the code automatically. If the formatting rules of the existing Smack source code and of the IDE used by the developer differ it may lead to patches that contain lots of unnecessary formatting changes which makes the code hard to review. Please set the save options to only auto-format the edited code to prevent this from occuring, or turn it off altogether when editing existing files and conform to the existing format, thus maintaining the ability to compare code between versions.
This is a proposal how all Smack source code should be formatted. Code contributions will not be declined if they don't match this styleguide but you are encouraged to follow this guide in order to create/preserve a maintainable code base that is easy to read for all developers.
A good set of rules is defined by the Java Code Conventions from Sun. If you have the option, choose this as the style format in your IDE. Most of the following guidelines match these conventions. There are a couple of places where they differ, so either would be considered acceptable.
Differences will be indicated.
Smack uses 4 spaces for indentation.
/**
* Indentation
*/
class Example {
int[] myArray = { 1, 2, 3, 4, 5, 6 };
int theInt = 1;
String someString = "Hello";
double aDouble = 3.0;
void foo(int a, int b, int c, int d, int e, int f) {
switch (a) {
case 0:
Other.doFoo();
break;
default:
Other.doBaz();
}
}
void bar(List v) {
for (int i = 0; i < 10; i++) {
v.add(new Integer(i));
}
}
}
enum MyEnum {
UNDEFINED(0) {
void foo() {
}
}
}
@interface MyAnnotation {
int count() default 1;
}All opening braces "{" should be on the same line as the declaration statement. The closing brace "}" starts a line by itself and is indented to match its corresponding opening statement.
void bar(List v) {
for (int i = 0; i < 10; i++) {
v.add(new Integer(i));
}
}Every code block (in "if" or "while" statements) must be enclosed by braces even if the block only contains one statement. By not doing this, one relies on the code layout for correctness. Bugs are easily introduced this way.
Blank lines should appear after each package, import, class, member and method declaration. There should be no successive empty lines. New lines Please note, this is a difference from the Java guidelines. There should be new lines before "do-while", "try-catch", "try-finally" and "else if" statements. "If" statements containing only a single statement should have new line.
/**
* If...else
*/
class Example {
void bar() {
do {
// do something
}
while (true);
try {
// do something
}
catch (Exception e) {
// exception handling
}
finally {
// cleanup
}
}
void foo2() {
if (true) {
return;
}
if (true) {
return;
}
else if (false) {
return;
}
else {
return;
}
}
void foo(int state) {
if (true) {
return;
}
if (true) {
return;
}
else if (false) {
return;
}
else {
return;
}
}
}The maximum line width should be 120 characters. Lines should be wrapped according to the following rules:
- Break after a comma.
- Break before an operator.
- Prefer higher-level breaks to lower-level breaks.
- Align the new line with the beginning of the expression at the same level on the previous line. If the above rules lead to confusing code or to code that's squished up against the right margin, just indent 8 spaces instead.
There are three different ways to use comments in Java. The documentation comments (/** ... /) and two ways for implementation comments ("/ ... /" and "// ... "). Documentation comments should be used to comment classes, methods and public fields. Non-public fields and multi-line comments within a method should use the "/ ... */" implementation comments. Single-line comments and trailing comments should use the "// ... " implementation comments.
/**
* Class Example
*/
public class Example {
/**
* Foo constant
*/
public static final String THE_FOO = "FOO";
/* foo */
private String foo;
/**
* Constructor
*/
public Example() {
// some initialization code
}
/**
* This method does something
*/
public int doThings() {
/*
* heavy documentation about lots of
* stuff done in this method
*/
...
if (condition) {
return 1; // trailing comment 1
}
else {
return -1; // trailing comment -1
}
}
}Smack source contains a code style configuration for eclipse which is based on the built-in Java code style with slight modifications.
Instance variables should be initialized in their declaration if possible. This improves readability of the code and reduces code in the constructor. You can omit explicitly initializing the value if the default initialization value for that type is the desired one.
Variable names should be a good compromise between being self-explaining and being short. Common local variables such as primitive variables for loop iteration or condition testing may be named with a short name that matches their class name.
int i;
String s;
File f;
float f;
double d;
Iterator<...> itTest whether you can return from a method instead of testing whether you should execute a block of code.
Instead of:
public void ... {
...
if (condition) {
// long block of code
}
}write:
public void ... {
...
if (!condition) {
return;
}
// long block of code
}At least all public methods should be fully documented including the JavaDoc Tags for the parameters, the return value and possible exceptions.
Since Smack is a network API, most methods throw the following excpetions:
-
XMPPException.XMPPErrorException- In case an XMPP protocol error occurs -
SmackException.NotConnectedException- In case the connection is not or no longer connected -
SmackException.NoResponseException- In case there was no response to our request after the reply timeout -
InterruptedException- In case the thread calling the method got interrupted
But sometimes you need to use methods which do not throw any type of subtype of those exceptions. In this case, wrap the Exception into a SmackException like this:
try {
...
}
catch (TimeoutException e) {
throw new SmackException("something went wrong", e);
}Also avoid catching exceptions by:
catch (Exception e) {
...
}This will not only catch all checked but also all non-checked and runtime exceptions (e.g. IllegalArgumentException, NullPointerException, etc.) which typically indicate problems in the application code and should not be hidden.
Each file must have a license header stating that the code is under the Apache License 2.0
/**
*
* Copyright XXXX Your Name
*
* All rights reserved. Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/org.jivesoftware.smack
Contains the core classes of Smack to create connections to XMPP servers, to send and receive Packets.
org.jivesoftware.smackx
Contains packages for implementations of the XEP specifications.
Each extension should have its own subpackage named according to the feature it implements: org.jivesoftware.smackx.[featureName]. If the feature is a XEP and that XEP has an assigned nickname (Usually found in the XEP under "Appendix A, Short Name") then use this short name as 'featureName'. For example code for "XEP-0172: User Nicknames" should go entirely in org.jivesoftware.smackx.nick
org.jivesoftware.smackx.[featureName].packet In order to implement a feature it is often required to create subclasses of org.jivesoftware.smack.packet.IQ which should be located in a subpackage of the feature package named "packet".
org.jivesoftware.smackx.[featureName].provider The classes that parse the XML representation into a Java object are called providers and are subclasses of org.jivesoftware.smack.provider.IQProvider or org.jivesoftware.smack.provider.PacketExtensionProvider. These providers should be located in a subpackage of the feature package named "provider".
Only classes and methods that are useful to developers using Smack should be made public. All fields should be private and only be accessible via getter and setter methods. Other methods that are only useful for the internal logic of the feature should be made private or protected.
Document concurrency related behavior. It should be clear if a particular class is intended to be thread safe, or not. Use the JCIP-based annotations to do this. Apart from documenting the code, static analyzers (such as FindBugs) can make use these annotations, allowing for better analysis of the code.
@ThreadSafe
public class Example {
// thread safe methods
}Prefer immutable objects. Immutable objects are by definition thread-safe. Some steps in making Objects immutable are:
- the class is final.
- the object can be constructed completely in one step (constructor or builder pattern)
- the object has no methods that change its state
- all fields are immutable.
To convert a primitive to a String use String.valueOf(...). Don't use "" + i.
To convert from a primitive use Integer.parseInt(...) and similar.
Don't use StringBuffer unless you need threadsafety, use StringBuilder instead. Similarly don't use Vector, use ArrayList.
Code to interfaces instead of specific implementations (Collection or List instead of ArrayList). Never subclass Thread, always pass a Runnable into the constructor instead.
If equals is implemented in a class it is MANDATORY to implement hashCode as well, otherwise HashSets and other classes which rely on the contract of hashCode will not work. IDEs like Eclipse provide generators to implement equals and hashCode.
Use enum instead of a bunch of static final int fields. Then you have type safety for related constants and more readable debugging output is possible because you get meaningful names instead of "magic" numbers.
Avoid the use of static methods and attributes if possible. This is not good object oriented programming style and it also makes testing your code more difficult. Testing
You should always write unit tests for your code to verify the behavior of your classes and methods and to make it easier for reviewers to understand what your code does and that you thought about some border cases.
For example if you have found a bug in Smack you should write a test that verifies that bug then fix the bug and run the test again to see that it doesn't fail anymore.
Writing test for all features and bugs verifies the correctness of the code and prevents regressions that might be caused by other changes and fixes.
Unit tests should be written for all bug fixes and new features, as they can easily be run for continuous integration and thus can be used to determine whether a build passes or fails. They are also easily written to test edge conditions and can guarentee inputs and outputs.
The folder "test" contains test that are running against a local or remote XMPP server. All test cases subclass SmackTestCase which provides a pre-configured execution context. By overwriting getMaxConnections you can specify how many connection you want to use in your test. Invoking getConnection(int index) returns the preconfigured connection.
public class ExampleTest extends SmackTestCase {
public ExampleTest(String arg0) {
super(arg0);
}
public void testSomething() {
Connection initiator = getConnection(0);
Connection target = getConnection(1);
// send some packets between connections
...
// verify
...
}
protected int getMaxConnections() {
return 2;
}
}The tests can be configured by editing the file "test/config/test-case.xml". Here you can set the host and port of the XMPP server and a prefix for the usernames of the users that will be automatically created for the tests.
Note: If you want to run the tests from Eclipse make sure that the resources are in the class path. Otherwise test may fail because Smack is not initialized correctly.
Gradle provides a target for the unit tests
gradle test
There are tools for java that can calculate the code accessed by unit tests like Emma, Cobertura or the Eclipse plugin EclEmma. A good test suite should cover about 80% of the code that you have written.
Another good tool to prevent coding errors is FindBugs which comes as a commandline tool or as Eclipse plugin. It analyzes the code and warns you about potential programming errors.
Smack is hosted in a git repository. The official URL of that git repository is: https://github.com/igniterealtime/Smack
In order to clone the repository use:
git clone https://github.com/igniterealtime/Smack.git
After you have cloned the git repository, create a new branch based on master. Use either the Smack issue key related to your patch, i.e. if your code fixes FOO-123 then name the branch "foo123", or choose a meaningful branch name (preferred). Small changes and fixes require usually only one commit, but if you are working on a bigger change, it may makes sense to organize your work in multiple commits.
Standard commit best practices apply:
- Only touch code that you need to change. Commits full of non-semantic (e.g. whitespace) changes are terrible to review
- Make separate commits for logically separate changes.
- Git commit messages should follow the 50/72 formatting rule.
- Don't put the JIRA issue in the commit's subject, i.e. the first line. It just consumes precious chars without a real benefit (e.g. when you look just the commit subject, the issue key tells you nothing).
- Describe your changes well.
- Style check your changes.
- Don't get discouraged. Re-submit.
- The first line of the commit message must always be 50 characters or less and it should be followed by a blank line
- Make sure that the commit does not contain any whitespace errors
The following resources are also helpful to create good git commits:
- ProGit Book - 5.2 Distributed Git - Contributing to a Project
- OpenStack Git Commit Good Practice
- Linux Kernel - Submitting Patches
- Git - Submitting Patches
Once your code is written, committed to your branch and tested (run the unit tests!) you can send a pull request against the main repository. Do not ask for your branch to get pulled into master! Instead create a pull request for the same branch name that you used.
If you can't create a pull request for whatever reason, you can also send patches created with git format-patch and simply attach the generated files to your forum post. Use this method only if necessary.
Now the only thing left to do is starting a discussion in the Smack Developers Board.
Every commit is up for review at least 2 weeks, where at least one Smack developer will review the commit.
It's up to the project lead to decide what goes when into the master branch, therefore it's the only instance that will merge/rebase code into master. All development work happens in branches.
Gradle is able to create the Smack projects for you with
gradle eclipse
Now you can import the Smack projects into Eclipse. Make sure to have "Search for nested subprojects" checked when importing!
After importing activate the provided Eclipse code formatter found under resources/eclipse/smack_formatter.xml
Happy coding!