Skip to content

obask/sandbox

BranchesTags

Folders and files

NameName
Last commit message
Last commit date

Latest commit

 

History

3 Commits
.gitignore
.gitignore
 
 
README.md
README.md
 
 

Repository files navigation

Opinionated Java Coding Standards

The purpose of the 47 Degrees Java Coding Standards is to create a collaboration baseline; helpful for scenarios where many people are creating, modifying, and contributing to the same project. The end goal is unity, consistency, and the notion that a single entity worked on the project.

Java applications developed at 47 Degrees should follow the Code Conventions for the Java Programming Language

47 Degrees uses the Java Programming Language and Java Related Technologies in different scenarios and applications. As a norm 47 Degrees follows industry standards and notifying of any deviation of this document from these standards will be welcomed.

Table of Contents generated with DocToc

0. Prerequirements

TODO: add initialization part TODO: http://geosoft.no/development/javastyle.html TODO: group and rearrange

0.1. Follow IDEA default settings and code formatting

Use following key bindings to format code: Ctrl + Cmd + L To format imports: Ctrl + Alt + O

0.2. Turn off star imports

Settings > Editor > Code Style > Java > Imports > Class count to use import with '*' Set it to a high value, e.g. 77.

Wildcard imports make the source of an imported class less clear. They also tend to hide a high class fan-out. See also texas imports

:::java
// Bad.
//   - Where did Foo come from?
import com.twitter.baz.foo.*;
import com.twitter.*;

interface Bar extends Foo {
  ...
}

// Good.

    import com.twitter.baz.foo.BazFoo;
    import com.twitter.Foo;

    interface Bar extends Foo {
      ...
    }

0.3. Install Lombok plugin

Get familiar with Lombok library and use it in your projects. (https://projectlombok.org) Alternative: Google AutoValue library, but it doesn't follow DRY principle.

Add the Lombok IntelliJ plugin to add lombok support IntelliJ:

1. General

  • All code, regardless of who touches it, should look like a single person created it.

POLA: Principle of least astonishment

(https://en.wikipedia.org/wiki/Principle_of_least_astonishment) Minimize number of assumtions needed to understand the code. Do not:

  • assume that all item ids are numbers from 1 to n
  • assume that ZonedDateTime is in UTC
  • JSON attributes are sorted
  • no one will change this piece of code and parameters order
  • while loop with break will stop at some point(TODO move out)

Use focal points, imagine how it would look like 6 months from now. In game theory, a focal point (also called Schelling point) is a solution that people will tend to use in the absence of communication. Example: "Tomorrow you have to meet a stranger in NYC. Where and when do you meet them? Schelling asked a group of students this question, and found the most common answer was: noon at Grand Central Terminal."

LoD: Principle of least knowledge

(https://en.wikipedia.org/wiki/Law_of_Demeter)

Recommended reading

Design principles

DRY: (https://en.wikipedia.org/wiki/Don%27t_repeat_yourself)

SRP: (https://en.wikipedia.org/wiki/Single_responsibility_principle)

2. Naming

2.1. Should package names be singular or plural?

Use the plural for packages with homogeneous contents and the singular for packages with heterogeneous contents. TBD... (https://softwareengineering.stackexchange.com/questions/75919/should-package-names-be-singular-or-plural)

2.1. Variables and methods

  • "is" prefix should be used for boolean variables and methods: isEncoded, isRight;
  • start counting variables with "n": nPoints, nPeople;
  • do not use short names: ptsArr -> pointsArray;
  • do not put "get" everywhere: getSize -> size;
  • Generic variables should have the same name as their type: Country -> country;

Extremely short variable names should be reserved for instances like loop indices.

:::java
// Bad.
//   - Field names give little insight into what fields are used for.
class User {
  private final int a;
  private final String m;

  ...
}

// Good:

    class User {
      private final int ageInYears;
      private final String maidenName;

      ...
    }

Include units in variable names

:::java
// Bad.
long pollInterval;
int fileSize;

// Good:

    long pollIntervalMs;
    int fileSizeGb.

    // Better.
    //   - Unit is built in to the type.
    //   - The field is easily adaptable between units, readability is high.
    Amount<Long, Time> pollInterval;
    Amount<Integer, Data> fileSize;

Don't embed metadata in variable names

A variable name should describe the variable's purpose. Adding extra information like scope and type is generally a sign of a bad variable name.

Avoid embedding the field type in the field name.

:::java
// Bad.
Map<Integer, User> idToUserMap;
String valueString;

// Good:

    Map<Integer, User> usersById;
    String value;

Also avoid embedding scope information in a variable. Hierarchy-based naming suggests that a class is too complex and should be broken apart.

:::java
// Bad.
String _value;
String mValue;

// Good:

    String value;

Composition Over Inheritance

Why: Less coupling between classes. Using inheritance, subclasses easily make assumptions, and break LSP.

How: Test for LSP (substitutability) to decide when to inherit. Compose when there is a “has a” (or “uses a”) relationship, inherit when “is a”.

(https://blogs.msdn.microsoft.com/thalesc/2012/09/05/favor-composition-over-inheritance/)

Constrain arguments by using type safe enumerations

Correct:

public enum Options {
	YES, NO
}

Incorrect:

String yes = "YES";
String no = "NO";

Convert to a builder if 3+ parameters have the same type

Correct:

@Builder
public class Person {
  private String name;
  private String secondName;
  private String nickname;
  private int age;
}

Person person = Person.builder().name("James").secondName("Gosling").nickname("hacker").age(30).build();

Incorrect:

@AllArgsConstructor
public class BuilderExample {
  private String name;
  private String secondName;
  private String nickname;
  private int age;
}

Person person = new Person("James", "hacker", "Gosling", 30);

Prefer explicit and value types over default types

Correct:

@AllArgsConstructor
public class Person {
  private Name name;
  private Age age;
  private SchoolId school;
}

Incorrect:

@AllArgsConstructor
public class BuilderExample {
  private String name;
  private String age;
  private String school;
}

Provide fields using constructor parameters, not setters

  • use lombok annotations: @NoArgsConstructor, @RequiredArgsConstructor, @AllArgsConstructor
  • use @NonNull and final for every field to make them more visible
  • place parameters to the top of the class prior to static fieds

Correct:

@RequiredArgsConstructor(staticName = "of")
public class Person {
    @NonNull
    private final Name name;
    @NonNull
    private final Age age;
    @NonNull
    private final SchoolId school;

    public static final BEAN_NAME = "Person";
    // ...
}

Incorrect:

public class Person {

    public static final BEAN_NAME = "Person";

    private Name name;
    private Age age;
    private SchoolId school;

    public void setName(Name name) {
        name = name;
    }
}

4.10. Name return values result

Consider using 'result' as the name for the returned variable. This eases the pain when debugging and increases code legibility.

public Object doSomething() {
	Object result = null;
	if (something) {
		result = new Object();
	}
	return result;
}

4.6. Exceptions

Don't swallow exception, catch those you can recover or do something about, let the rest reach their destination

Correct:

	try {
		do();
	} catch (SomethingWeCanHandleException ex) {
		log.error(ex);
		notifyUser(ex);
	} finally {
		cleanUp();
	}
	public void doSomething() throws SomethingSomeoneElseCanHandleException {
	}

Incorrect:

	try {
		do();
	} catch (Throwable ex) {
		log.error(ex);
	} 
	try {
		do();
	} catch (SomethingWeCanHandleException ex) {
		//do nothing
	}

4.6.1. Do not catch Throwable and java.lang.Error!

Otherwise unforeseen bugs might creep away this way.

Besides, Throwable covers Error as well and that's usually no point of return. You don't want to catch/handle that, you want your program to die immediately so that you can fix it properly. Generally, never. However, sometimes you need to catch specific Errors.

If you're writing framework-ish code (loading 3rd party classes), it might be wise to catch LinkageErrors (no class def found, unsatisfied link, incompatible class change). I've also seen some stupid 3rd-party code throwing sublcasses of Errors, so you'll have to handle these either.

By the way, I'm not sure it isn't possible to recover from OutOfMemory.

4.6.2. Prefer unchecked exceptions

Correct:

private final int lineCount;
try {
    lineCount = LineCounter.countLines(sFileName);
} catch(IOException ex){
    throw new UncheckedIOException(ex);
}

4.6.3. Always close/release resources where they are created

AmazonS3 s3Client = new AmazonS3Client(new ProfileCredentialsProvider());        
S3Object object = s3Client.getObject(
                  new GetObjectRequest(bucketName, key));
try (InputStream objectData = object.getObjectContent()) {
    // Process the objectData stream.
}
AmazonS3 s3Client = new AmazonS3Client(new ProfileCredentialsProvider());        
S3Object object = s3Client.getObject(
                  new GetObjectRequest(bucketName, key));
InputStream objectData = object.getObjectContent();
try {
    // Process the objectData stream.
} finally {
    objectData.close();
}

Use annotations wisely

@Nullable

By default - disallow null. When a variable, parameter, or method return value may be null, be explicit about it by marking @Nullable. This is advisable even for fields/methods with private visibility.

    class Database {
      @Nullable private Connection connection;

      @Nullable
      Connection getConnection() {
        return connection;
      }

      void setConnection(@Nullable Connection connection) {
        this.connection = connection;
      }
    }

@VisibleForTesting

Sometimes it makes sense to hide members and functions in general, but they may still be required for good test coverage. It's usually preferred to make these package-private and tag with @VisibleForTesting to indicate the purpose for visibility.

Constants are a great example of things that are frequently exposed in this way.

:::java
// Bad.
//   - Any adjustments to field names need to be duplicated in the test.
class ConfigReader {
  private static final String USER_FIELD = "user";

  Config parseConfig(String configData) {
    ...
  }
}
public class ConfigReaderTest {
  @Test
  public void testParseConfig() {
    ...
    assertEquals(expectedConfig, reader.parseConfig("{user: bob}"));
  }
}

// Good.

    //   - The test borrows directly from the same constant.
    class ConfigReader {
      @VisibleForTesting static final String USER_FIELD = "user";

      Config parseConfig(String configData) {
        ...
      }
    }
    public class ConfigReaderTest {
      @Test
      public void testParseConfig() {
        ...
        assertEquals(expectedConfig,
            reader.parseConfig(String.format("{%s: bob}", ConfigReader.USER_FIELD)));
      }
    }

Time-dependence

Code that captures real wall time can be difficult to test repeatably, especially when time deltas are meaningful. Therefore, try to avoid new Date(), System.currentTimeMillis(), and System.nanoTime(). A suitable replacement for these is Clock; using Clock.SYSTEM_CLOCK when running normally, and FakeClock in tests.

Defensive programming

Avoid assert

We avoid the assert statement since it can be disabled at execution time, and prefer to enforce these types of invariants at all times.

Minimize visibility

In a class API, you should support access to any methods and fields that you make accessible. Therefore, only expose what you intend the caller to use. This can be imperative when writing thread-safe code.

:::java
public class Parser {
  // Bad.
  //   - Callers can directly access and mutate, possibly breaking internal assumptions.
  public Map<String, String> rawFields;

  // Bad.
  //   - This is probably intended to be an internal utility function.
  public String readConfigLine() {
    ..
  }
}

// Good.

    //   - rawFields and the utility function are hidden
    //   - The class is package-private, indicating that it should only be accessed indirectly.
    class Parser {
      private final Map<String, String> rawFields;

      private String readConfigLine() {
        ..
      }
    }

Favor immutability

Mutable objects carry a burden - you need to make sure that those who are able to mutate it are not violating expectations of other users of the object, and that it's even safe for them to modify.

:::java
// Bad.
//   - Anyone with a reference to User can modify the user's birthday.
//   - Calling getAttributes() gives mutable access to the underlying map.
public class User {
  public Date birthday;
  private final Map<String, String> attributes = Maps.newHashMap();

  ...

  public Map<String, String> getAttributes() {
    return attributes;
  }
}

// Good.

    public class User {
      private final Date birthday;
      private final Map<String, String> attributes = Maps.newHashMap();

      ...

      public Map<String, String> getAttributes() {
        return ImmutableMap.copyOf(attributes);
      }

      // If you realize the users don't need the full map, you can avoid the map copy
      // by providing access to individual members.
      @Nullable
      public String getAttribute(String attributeName) {
        return attributes.get(attributeName);
      }
    }

Be wary of null

Use @Nullable where prudent, but favor Optional over @Nullable. Optional provides better semantics around absence of a value.

Clean up with finally (TODO deduplicate)

Even if there are no checked exceptions, there are still cases where you should use try/finally to guarantee resource symmetry.

:::java
// Bad.
//   - Mutex is never unlocked.
mutex.lock();
throw new NullPointerException();
mutex.unlock();

// Good.

    mutex.lock();
    try {
      throw new NullPointerException();
    } finally {
      mutex.unlock();
    }

    // Bad.
    //   - Connection is not closed if sendMessage throws.
    if (receivedBadMessage) {
      conn.sendMessage("Bad request.");
      conn.close();
    }

    // Good.
    if (receivedBadMessage) {
      try {
        conn.sendMessage("Bad request.");
      } finally {
        conn.close();
      }
    }

Obey the Law of Demeter (LoD)

The Law of Demeter is most obviously violated by breaking the one dot rule, but there are other code structures that lead to violations of the spirit of the law.

In classes

Take what you need, nothing more. This often relates to texas constructors but it can also hide in constructors or methods that take few parameters. The key idea is to defer assembly to the layers of the code that know enough to assemble and instead just take the minimal interface you need to get your work done.

:::java
// Bad.
//   - Weigher uses hosts and port only to immediately construct another object.
class Weigher {
  private final double defaultInitialRate;

  Weigher(Iterable<String> hosts, int port, double defaultInitialRate) {
    this.defaultInitialRate = validateRate(defaultInitialRate);
    this.weightingService = createWeightingServiceClient(hosts, port);
  }
}

// Good.
class Weigher {
  private final double defaultInitialRate;

  Weigher(WeightingService weightingService, double defaultInitialRate) {
    this.defaultInitialRate = validateRate(defaultInitialRate);
    this.weightingService = checkNotNull(weightingService);
  }
}

If you want to provide a convenience constructor, a factory method or an external factory in the form of a builder you still can, but by making the fundamental constructor of a Weigher only take the things it actually uses it becomes easier to unit-test and adapt as the system involves.

In methods

If a method has multiple isolated blocks consider naming these blocks by extracting them to helper methods that do just one thing. Besides making the calling sites read less like code and more like english, the extracted sites are often easier to flow-analyse for human eyes. The classic case is branched variable assignment. In the extreme, never do this:

:::java
void calculate(Subject subject) {
  double weight;
  if (useWeightingService(subject)) {
    try {
      weight = weightingService.weight(subject.id);
    } catch (RemoteException e) {
      throw new LayerSpecificException("Failed to look up weight for " + subject, e)
    }
  } else {
    weight = defaultInitialRate * (1 + onlineLearnedBoost);
  }

  // Use weight here for further calculations
}

Instead do this:

:::java
void calculate(Subject subject) {
  double weight = calculateWeight(subject);

  // Use weight here for further calculations
}

private double calculateWeight(Subject subject) throws LayerSpecificException {
  if (useWeightingService(subject)) {
    return fetchSubjectWeight(subject.id)
  } else {
    return currentDefaultRate();
  }
}

private double fetchSubjectWeight(long subjectId) {
  try {
    return weightingService.weight(subjectId);
  } catch (RemoteException e) {
    throw new LayerSpecificException("Failed to look up weight for " + subject, e)
  }
}

private double currentDefaultRate() {
  defaultInitialRate * (1 + onlineLearnedBoost);
}

A code reader that generally trusts methods do what they say can scan calculate quickly now and drill down only to those methods where I want to learn more.

Don't Repeat Yourself (DRY)

For a more long-winded discussion on this topic, read here.

4. Follow Effective Java when appropriate

Presents the most practical, authoritative guidelines available for writing efficient, well-designed programs for the Java platform Completely updated for Java releases since 2008 Java programming paradigm has evolved significantly in last 5 years and new material covered in this edition is critical to modern Java programming

Thoroughly revised and updated to cover language and library features added in Java 7, 8, and 9, and recent trends in Java programming. Many new items have been added, including a chapter devoted to lambdas and streams. New topics include: --Functional interfaces, lambda expressions, method references, and streams --Default and static methods in interfaces --Type inference, including the diamond operator for generic types --The @SafeVarargs annotation --The try-with-resources statement --New library features such as java.time and the convenience factory methods for collections

5. SEI CERT Oracle Coding Standard for Java

Rule 00. Input Validation and Data Sanitization (IDS)

Rule 01. Declarations and Initialization (DCL)

Rule 02. Expressions (EXP)

Rule 03. Numeric Types and Operations (NUM)

Rule 04. Characters and Strings (STR)

Rule 05. Object Orientation (OBJ)

Rule 06. Methods (MET)

Rule 07. Exceptional Behavior (ERR)

Rule 08. Visibility and Atomicity (VNA)

Rule 09. Locking (LCK)

Rule 10. Thread APIs (THI)

Rule 13. Input Output (FIO)

Rule 14. Serialization (SER)

Rule 15. Platform Security (SEC)

Rule 16. Runtime Environment (ENV)

Rule 17. Java Native Interface (JNI)

Rec. 18. Concurrency (CON)

Rule 49. Miscellaneous (MSC)

About

Opinionated Java Coding Standards

Resources

Readme
Activity

Stars

1 star

Watchers

1 watching

Forks

1 fork
Report repository