×
Community Blog Common Java Code Defects and Solutions

Common Java Code Defects and Solutions

This article summarizes common code defects and solutions encountered in daily development to provide helpful insights.

By Xiniao

1

Problems

Null Pointer Exception (NPE)

NPE is perhaps the most common problem in programming languages, which Null's inventor, Tony Hoare, called a billion-dollar mistake. There is no built-in syntax for handling Null values in Java, but there are still some relatively elegant ways to help us avoid NPE.

• Use annotations such as JSR-305/jetbrain

  1. NotNull
  2. Nullable

2

By explicitly marking whether the value may be Null in the method parameters, return values, and fields, together with code inspection tools, you can avoid most NPE problems at the coding stage. I recommend that you use this annotation at least in common methods or external APIs, which can provide significant help to callers.

• Handle Method Chaining with Optional

Optional is derived from the Optional class in Guava and later built into the JDK in Java 8. Optional is generally used as the return value of a function, forcibly reminding the caller that the return value may not exist, and can gracefully handle null values through method chaining.

public class OptionalExample {

    public static void main(String[] args) {
        // Use the traditional method to deal with null value.
        User user = getUser();
        String city = "DEFAULT";
        if (user != null && user.isValid()) {
            Address address = user.getAddress();
            if (adress != null) {
                city = adress.getCity();
            }
        }
        System.out.println(city);

        // Use the Optional method.
        Optional<User> optional = getUserOptional();
        city = optional.filter(User::isValid)
                .map(User::getAddress)
            .map(Adress::getCity)
              .orElse("DEFAULT")
        System.out.println(city);
    }

    @Nullable
    public static User getUser() {
        return null;
    }

    public static Optional<User> getUserOptional() {
        return Optional.empty();
    }

    @Data
    public static class User {
        private Adress address;
        private boolean valid;
    }

    @Data
    public static class Address {
        private String city;
    }
}

• Replace a.equals(b) with Objects.equals(a,b)

The equals method is a high incidence of NPE. Using Objects.euqals to compare two objects can avoid NPE when any object is null.

• Use the Empty Object Pattern

The null object mode represents the default behavior mode when an object does not exist by replacing the non-existent situation with a special object.

Common example:

If Empty List is used instead of null, EmptyList can be traversed normally:

public class EmptyListExample {

    public static void main(String[] args) {
        List<String> listNullable = getListNullable();
        if (listNullable != null) {
            for (String s : listNullable) {
                System.out.println(s);
            }
        }

        List<String> listNotNull = getListNotNull();
        for (String s : listNotNull) {
            System.out.println(s);
        }
    }

    @Nullable
    public static List<String> getListNullable() {
        return null;
    }

    @NotNull
    public static List<String> getListNotNull() {
        return Collections.emptyList();
    }
}

Empty Policy

public class NullStrategyExample {

    private static final Map<String, Strategy> strategyMap = new HashMap<>();

    public static void handle(String strategy, String content) {
        findStrategy(strategy).handle(content);
    }

    @NotNull
    private static Strategy findStrategy(String strategyKey) {
        return strategyMap.getOrDefault(strategyKey, new DoNothing());
    }

    public interface Strategy {
        void handle(String s);
    }

    // If no corresponding policy is found, do nothing.
    public static class DoNothing implements Strategy {
        @Override
        public void handle(String s) {

        }
    }
}

Object Conversion

In business applications, our code structure is often multi-level, and the transformation of objects is often involved between different levels. Though it seems simple, it can be quite tedious and prone to errors.

Counterexample 1:

public class UserConverter {

    public static UserDTO toDTO(UserDO userDO) {
        UserDTO userDTO = new UserDTO();
        userDTO.setAge(userDO.getAge());
        // Problem 1: Assign a value to itself.
        userDTO.setName(userDTO.getName());
        return userDTO;
    }

    @Data
    public static class UserDO {
        private String name;
        private Integer age;
        // Problem 2: The new field is not assigned a value.
        private String address;
    }

    @Data
    public static class UserDTO {
        private String name;
        private Integer age;
    }
}

Counterexample 2:

public class UserBeanCopyConvert {

    public UserDTO toDTO(UserDO userDO) {
        UserDTO userDTO = new UserDTO();
        // Use reflection to copy different types of objects.
        //  1. Refactoring is not friendly. When you want to delete or modify the field of UserDO, you cannot know whether the field is depended on by other fields through reflection.
        BeanUtils.copyProperties(userDO, userDTO);
        return userDTO;
    }
}

• Using Mapstruct

Mapstruct uses compile-time code generation technology to automatically generate transformations and code based on annotations, input parameters and output parameters, and supports various advanced features, such as:

  1. The processing strategy of unmapped fields; the mapping problem is found at compile time;
  2. Reuse tool to facilitate field type conversion;
  3. Generate Spring component annotations and manage them through Spring.
  4. and so on;
@Mapper(
    componentModel = "spring",
    unmappedSourcePolicy = ReportingPolicy.ERROR,
    unmappedTargetPolicy = ReportingPolicy.ERROR,
    // The convert logic depends on DateUtil for date conversion.
    uses = DateUtil.class
)
public interface UserConvertor  {

    UserDTO toUserDTO(UserDO userDO);

    @Data
    class UserDO {
        private String name;
        private Integer age;
        //private String address;
        private Date birthDay;
    }

    @Data
    class UserDTO {
        private String name;
        private Integer age;
        private String birthDay;
    }

}

public class DateUtil {
    public static String format(Date date) {
        SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd");
        return simpleDateFormat.format(date);
    }
}

Sample code:

@RequiredArgsConstructor
@Component
public class UserService {
    private final UserDao userDao;
    private final UserCovertor userCovertor;

    public UserDTO getUser(String userId){
        UserDO userDO = userDao.getById(userId);
        return userCovertor.toUserDTO(userDO);
    }
}

Compile-time verification:

3

The generated code:

@Generated(
    value = "org.mapstruct.ap.MappingProcessor",
    date = "2023-12-18T20:17:00+0800",
    comments = "version: 1.3.1.Final, compiler: javac, environment: Java 11.0.12 (GraalVM Community)"
)
@Component
public class UserConvertorImpl implements UserConvertor {

    @Override
    public UserDTO toUserDTO(UserDO userDO) {
        if ( userDO == null ) {
            return null;
        }

        UserDTO userDTO = new UserDTO();

        userDTO.setName( userDO.getName() );
        userDTO.setAge( userDO.getAge() );
        userDTO.setBirthDay( DateUtil.format( userDO.getBirthDay() ) );

        return userDTO;
    }
}

Thread Safety

The memory model of JVM is very complicated and difficult to understand. Java Concurrency in Practice tells you that unless you are very familiar with the thread safety principle of JVM, you should strictly abide by the basic Java thread safety rules and use the thread safety classes and keywords built in Java.

• Use Thread-safe Classes Proficiently

ConcurrentHashMap

Counterexample:

The map.get and map.put operations are non-atomic operations. Inconsistency may occur when multiple threads are concurrently modified. For example, thread A calls the append method. In line 6, thread B deletes the key.

public class ConcurrentHashMapExample {
    private Map<String, String> map = new ConcurrentHashMap<>();

    public void appendIfExists(String key, String suffix) {
        String value = map.get(key);
        if (value != null) {
            map.put(key, value + suffix);
        }
    }
}

Positive example:

public class ConcurrentHashMapExample {
    private Map<String, String> map = new ConcurrentHashMap<>();

    public void append(String key, String suffix) {
        // Use computeIfPresent atomic operation
        map.computeIfPresent(key, (k, v) -> v + suffix);
    }
}

• Guarantee Atomicity of Changes

Counterexample:

@Getter
public class NoAtomicDiamondParser {

    private volatile int start;

    private volatile int end;

    public NoAtomicDiamondParser() {
        Diamond.addListener("dataId", "groupId", new ManagerListenerAdapter() {
            @Override
            public void receiveConfigInfo(String s) {
                JSONObject jsonObject = JSON.parseObject(s);
                start = jsonObject.getIntValue("start");
                end  = jsonObject.getIntValue("end");
            }
        });
    }
}

public class MyController{

    private final NoAtomicDiamondParser noAtomicDiamondParser;

    public void handleRange(){
        // The old value is read by the end and the new value is read by the start. The start value may be greater than the end value.
        int end = noAtomicDiamondParser.getEnd();
        int start = noAtomicDiamondParser.getStart();
    }
}

Positive example:

@Getter
public class AtomicDiamondParser {

    private volatile Range range;

    public AtomicDiamondParser() {
        Diamond.addListener("dataId", "groupId", new ManagerListenerAdapter() {
            @Override
            public void receiveConfigInfo(String s) {
                range = JSON.parseObject(s, Range.class);
            }
        });
    }

    @Data
    public static class Range {
        private int start;
        private int end;
    }
}

public class MyController {

    private final AtomicDiamondParser atomicDiamondParser;

    public void handleRange() {
        Range range = atomicDiamondParser.getRange();
        System.out.println(range.getStart());
        System.out.println(range.getEnd());
    }
}

• Using Immutable Objects

When an object is immutable, there is no thread safety problem in the object. If you need to modify the object, you must create a new object. This method is suitable for simple value object types. Common examples are String and BigDecimal in Java. For the above example, we can also design the Range as a generic value object.

Positive example:

@Getter
public class AtomicDiamondParser {

    private volatile Range range;

    public AtomicDiamondParser() {
        Diamond.addListener("dataId", "groupId", new ManagerListenerAdapter() {
            @Override
            public void receiveConfigInfo(String s) {
                JSONObject jsonObject = JSON.parseObject(s);
                int start = jsonObject.getIntValue("start");
                int end  = jsonObject.getIntValue("end");
                range = new Range(start, end);
            }
        });
    }

    // The lombok annotation guarantees the immutability of the Range class.
    @Value
    public static class Range {
        private int start;
        private int end;
    }
}

• Correctness Takes Precedence over Performance

Don't give up using keywords such as synchronized and volatile, or use some unconventional writing because you are worried about the performance.

Counterexample: double-checked lock:

class Foo { 
  // The volatile keyword is missing.
  private Helper helper = null;
  public Helper getHelper() {
    if (helper == null) 
      synchronized(this) {
        if (helper == null) 
          helper = new Helper();
      }    
    return helper;
    }
}

In the preceding example, adding the volatile keyword to the helper field ensures thread safety in Java 5 and later versions.

Positive example:

class Foo { 
  private volatile Helper helper = null;
  public Helper getHelper() {
    if (helper == null) 
      synchronized(this) {
        if (helper == null) 
          helper = new Helper();
      }    
    return helper;
    }
}

Positive example 3 (recommended):

class Foo { 
  private Helper helper = null;
  public synchronized Helper getHelper() {
      if (helper == null) 
          helper = new Helper();
      }    
    return helper;
}

An incomplete Diamond Parser:

/**
 * Omit other logic such as exception handling.
 */
@Getter
public class DiamondParser {

    // The volatile keyword is missing.
    private Config config;

    public DiamondParser() {
        Diamond.addListener("dataId", "groupId", new ManagerListenerAdapter() {
            @Override
            public void receiveConfigInfo(String s) {
                config = JSON.parseObject(s, Config.class);
            }
        });
    }

    @Data
    public static class Config {
        private String name;
    }
}

This Diamond writing method may never have had online problems, but it does not conform to the JVM thread safety principle. One day in the future when your code runs on another JVM implementation, there may be problems.

Improper Use of the Thread Pool

Counterexample 1:

public class ThreadPoolExample {

    // The thread pool without any restrictions is convenient to use. However, when request peaks arrive, a large number of threads may be created, causing the system to crash.
    private static Executor executor = Executors.newCachedThreadPool();

}

Counterexample 2:

public class StreamParallelExample {

    public List<String> batchQuery(List<String> ids){
        // It looks elegant, but the queue size of ForkJoinPool is unlimited and the number of threads is small. If the IDs list is large, OOM may be caused.
        // ParallelStream is more suitable for computing-intensive tasks. Do not make remote calls during tasks
        return ids.parallelStream()
            .map(this::queryFromRemote)
            .collect(Collectors.toList());
    }

    private String queryFromRemote(String id){
       // Query from the remote
    }
}

• Manually Create a Thread Pool

Positive example:

public class ManualCreateThreadPool {

    // Manually create a thread pool with limited resources.
    private Executor executor = new ThreadPoolExecutor(10, 10, 1, TimeUnit.MINUTES, new ArrayBlockingQueue<>(1000),
        new ThreadFactoryBuilder().setNameFormat("work-%d").build());
}

Improper Handling of Exceptions

Like NPE, exception handling is also a problem we need to face every day, but what often appears in many codes are:

Counterexample 1:

Repeated and tedious exception-handling logic

@Slf4j
public class DuplicatedExceptionHandlerExample {

    private UserService userService;

    public User query(String id) {
        try {
            return userService.query(id);
        } catch (Exception e) {
            log.error("query error, userId: {}", id, e);
            return null;
        }
    }

    public User create(String id) {
        try {
            return userService.create(id);
        } catch (Exception e) {
            log.error("query error, userId: {}", id, e);
            return null;
        }
    }
}

Counterexample 2:

Exceptions are swallowed or some information is lost

@Slf4j
public class ExceptionShouldLogOrThrowExample {

    private UserService userService;

    public User query(String id) {
        try {
            return userService.query(id);
        } catch (Exception e) {
            // The exception is annexed and the problem is hidden.
            return null;
        }
    }

    public User create(String id) {
        try {
            return userService.create(id);
        } catch (Exception e) {
            // The stack is lost and it is difficult to locate the problem later.
            log.error("query error, userId: {}, error: {}", id,e.getMessage() );
            return null;
        }
    }
}

Counterexample 3:

An unknown exception is thrown externally, causing the caller to fail to serialize.

public class OpenAPIService {

    public void handle(){
        // The HSF service throws an exception that is not defined by the client. The caller fails to deserialize the HSF service.
        throw new InternalSystemException("");
    }
}

• Unified Exception Handling Through AOP

  1. Avoid throwing unknown exceptions to callers. Convert unknown exceptions to Results or common exception types.
  2. Unified exception log printing and monitoring.

• Handle Checked Exceptions

Checked Exceptions are exceptions that must be handled during the compiling, that is, non-RuntimeException type exceptions. However, Java Checked Exception places a certain burden on the caller of the interface, causing the exception declaration to be passed layer by layer. If the top layer can handle the exception, we can avoid Checked Exception through lombok's @SneakyThrows annotation.

4

• Try Catch Thread Logic

Counterexample:

@RequiredArgsConstructor
public class ThreadNotTryCatch {
    private final ExecutorService executorService;
    public void handle() {
        executorService.submit(new Runnable() {
            @Override
            public void run() {
                // The exception is not caught. The thread exits directly, and the exception information is lost.
                remoteInvoke();
            }
        });
    }
}

Positive example:

@RequiredArgsConstructor
@Slf4j
public class ThreadNotTryCatch {
    private final ExecutorService executorService;

    public void handle() {
        executorService.submit(new Runnable() {
            @Override
            public void run() {
                try {
                    remoteInvoke();
                } catch (Exception e) {
                    log.error("handle failed", e);
                }
            }
        });
    }
}

• Handling of Special Exceptions

InterruptedException is usually an interruption signal initiated by the upper scheduler. For example, if a task times out, the scheduler interrupts the task by setting the thread to interrupted. We should not ignore such exceptions after the catch, but should throw them upward or set the current thread to interrupted.

Counterexample:

public class InterruptedExceptionExample {
    private ExecutorService executorService = Executors.newSingleThreadExecutor();

    public void handleWithTimeout() throws InterruptedException {
        Future<?> future = executorService.submit(() -> {
            try {
                // Sleep simulates the processing logic.
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                System.out.println("interrupted");
            }
            System.out.println("continue task");
            // The exception is ignored. Continue to process.
        });
        // Wait for the task result, and interrupt if the time exceeds 500ms.
        Thread.sleep(500);
        if (!future.isDone()) {
            System.out.println("cancel");
            future.cancel(true);
        }
    }
}

• Avoid Catch Errors

Do not swallow errors. Error design is different from exception. Generally, errors should not be caught, let alone swallowed. For example, OOM may occur at any code location. If an error is annexed and the program continues to run, the start and end of the following code cannot be consistent.

public class ErrorExample {

    private Date start;

    private Date end;

    public synchronized void update(long start, long end) {
        if (start > end) {
            throw new IllegalArgumentException("start after end");
        }
        this.start = new Date(start);
        // If an OOM error occurs at the new date (end), the start value may be greater than the end value.
        this.end = new Date(end);
    }
}

Spring Bean Implicit Dependency

Counterexample 1: SpringContext as a static variable

UserController and SpringContextUtils classes have no dependencies. SpringContextUtils.getApplication() may return null. In addition, the initialization order between Spring-independent beans is uncertain. Although the current initialization order may happen to meet expectations, it may change later.

@Component
public class SpringContextUtils {

    @Getter
    private static ApplicationContext applicationContext;

    public SpringContextUtils(ApplicationContext context) {
        applicationContext = context;
    }
}

@Component
public class UserController {

    public void handle(){
        MyService bean = SpringContextUtils.getApplicationContext().getBean(MyService.class);
    }
}

Counterexample 2: Switch is registered in Spring Bean but is read statically.

@Component
public class SwitchConfig {

    @PostConstruct
    public void init() {
        SwitchManager.register("appName", MySwitch.class);
    }

    public static class MySwitch {
        @AppSwitch(des = "config", level = Switch.Level.p1)
        public static String config;
    }
}

@Component
public class UserController{

    public String getConfig(){
        //The UserController and SwitchConfig classes have no dependency. MySwitch.config may not be initialized yet.
        return MySwitch.config;
    }
}

Guaranteed initialization order by SpringBeanFactory:

public class PreInitializer implements BeanFactoryPostProcessor, PriorityOrdered {

  @Override
  public int getOrder() {
    return Ordered.HIGHEST_PRECEDENCE;
  }

  @Override
  public void postProcessBeanFactory(
    ConfigurableListableBeanFactory beanFactory) throws BeansException {
       try {
        SwitchManager.init(application name, switch class.class);
      } catch (SwitchCenterException e) {
        // If an error is thrown here, it is best to block the startup of the program to avoid problems caused by the switch not reading the persistent value.
    } catch (SwitchCenterError e) {
        System.exit(1);
    }
    }
}
@Component
public class SpringContextUtilPostProcessor implements BeanFactoryPostProcessor, PriorityOrdered, ApplicationContextAware {

    private ApplicationContext applicationContext;

    @Override
    public int getOrder() {
        return Ordered.HIGHEST_PRECEDENCE;
    }

    @Override
    public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory)
            throws BeansException {
        SpringContextUtils.setApplicationContext(applicationContext);
    }

    @Override
    public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
        this.applicationContext = applicationContext;
    }
}

Memory/Resource Leakage

Although JVM has a garbage collection mechanism, it does not mean that memory leaks do not exist. Generally, memory leaks occur in scenarios where objects cannot be released for a long time, such as static collections, cached data in memory, run-time class generation technology, etc.

• Use LoadingCache Instead of Global Map

@Service
public class MetaInfoManager {

    // For a small amount of metadata, it does not seem to be a big problem to put it into the memory. However, if the amount of metadata subsequently increases, a large number of objects cannot be released from the memory, resulting in memory leakage.
    private Map<String, MetaInfo> cache = new HashMap<>();

    public MetaInfo getMetaInfo(String id) {
        return cache.computeIfAbsent(id, k -> loadFromRemote(id));
    }

    private LoadingCache<String, MetaInfo> loadingCache = CacheBuilder.newBuilder()
        // Set the maximum size or expiration time of the loadingCache to limit the number of cache entries
        .maximumSize(1000)
        .build(new CacheLoader<String, MetaInfo>() {
            @Override
            public MetaInfo load(String key) throws Exception {
                return loadFromRemote(key);
            }
        });

    public MetaInfo getMetaInfoFromLoadingCache(String id) {
        return loadingCache.getUnchecked(id);
    }

    private MetaInfo loadFromRemote(String id) {
        return null;
    }

    @Data
    public static class MetaInfo {
        private String id;
        private String name;
    }
}

• Use Runtime Class Generation Techniques with Caution

Cglib, Javasisit, or Groovy scripts create temporary classes at runtime. JVM is very strict about class recycling conditions, so these temporary classes will not be recycled for a long time until FullGC is triggered.

• Use Try With Resource

Use the Java 8 try with Resource syntax:

public class TryWithResourceExample {

    public static void main(String[] args) throws IOException {
        try (InputStream in = Files.newInputStream(Paths.get(""))) {
            // read
        }
    }
}

Performance

URL hashCodeeuqals method

The implementation of the hashCode method and equals method of URL involves the resolution of the IP address of the domain name, so it is possible to trigger a remote call in a data structure such as a map or in an explicit call. Using a URI instead of a URL can avoid this problem.

Counterexample 1:

public class URLExample {
    public void handle(URL a, URL b) {
        if (Objects.equals(a, b)) {

        }
    }
}

Counterexample 2:

public class URLMapExample {

    private static final Map<URL, Object> urlObjectMap = new HashMap<>();

}

Loop remote call:

public class HSFLoopInvokeExample {

    @HSFConsumer
    private UserService userService;

    public List<User> batchQuery(List<String> ids){
        // Use the batch interface or limit the batch size.
       return ids.stream()
            .map(userService::getUser)
            .collect(Collectors.toList());
    }
}

• Understand Common Performance Metrics&Bottlenecks

Understanding some basic performance metrics helps us accurately assess the performance bottleneck of the current problem. For example, if you set the field to volatile, you need to read the main memory every time. The performance of reading the main memory is about nanoseconds, which is unlikely to become a performance bottleneck in an HSF call. Compared with ordinary operations, reflection reads more memory, which is generally considered to have poor performance, but it is also unlikely to become a performance bottleneck in an HSF call.

5

In server development, performance bottlenecks generally revolve around:

  • Printing a large number of logs
  • Serialization of large objects
  • Network calls: remote calls such as HSF and HTTP
  • Database operations

• Use Professional Performance Testing Tools to Evaluate Performance

Don't try to implement crude performance testing by yourself. During the test code running, there may be unexpected optimizations in the compiler, the JVM, or at all levels of the operating system, resulting in overly optimistic test results. It is recommended to use professional tools like JMH and Arthas flame graph to do performance testing.

Counterexample:

public class ManualPerformanceTest {

    public void testPerformance() {
        long start = System.currentTimeMillis();
        for (int i = 0; i < 1000; i++) {
            // Mutiply here has no side effects and may be killed after optimization.
            mutiply(10, 10);
        }
        System.out.println("avg rt: " + (System.currentTimeMillis() - start) / 1000);
    }

    private int mutiply(int a, int b) {
        return a * b;
    }
}

Positive example:

Use the flame graph

6

Positive example 2:

Evaluate performance by using JMH

@Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
@Fork(3)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class JMHExample {

    @Benchmark
    public void testPerformance(Blackhole bh) {
        bh.consume(mutiply(10, 10));
    }

    private int mutiply(int a, int b) {
        return a * b;
    }
}

Spring Transaction Issues

• Note Scenarios Where the Transaction Annotation Becomes Invalid

When a Spring bean annotated with @Transactional is injected, Spring will inject the object that was proxied by the transaction instead of the original object.

However, if the annotation method is called by another method in the same object, the call cannot be intervened by Spring, and the natural transaction annotation is invalid.

@Component
public class TransactionNotWork {

    public void doTheThing() {
        actuallyDoTheThing();
    }

    @Transactional
    public void actuallyDoTheThing() {
    }
}

References

  1. NULL: The Billion Dollar Mistake: https://www.infoq.cn/article/uyyos0vgetwcgmo1ph07
  2. Double-checked Lock Failure Declaration: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
  3. Latency Numbers Every Programmer Should Know: https://colin-scott.github.io/personal_website/research/interactive_latency.html


Disclaimer: The views expressed herein are for reference only and don't necessarily represent the official views of Alibaba Cloud.

0 1 0
Share on

Alibaba Cloud Community

1,062 posts | 262 followers

You may also like

Comments