20 Code Smells That Drive Developers Crazy (And How to Fix Them)

Preface

Today we discuss an interesting topic: how to write code that drives developers crazy.

At first glance the title may look like clickbait, but the article is packed with practical technical advice.

Have you ever read someone else's code and felt frustrated, angry, or ready to scream?

Let's look at the 20 kinds of code that can drive you crazy and see if you have encountered any of them.

1. Ignoring Code Formatting

Formatting may seem abstract, so here are a few examples showing the effect of poor formatting.

1.1 Missing Spaces

Sometimes necessary spaces are omitted, for example:

@Service
@Slf4j
public class TestService1{
public void test1(){
addLog("test1");
if(condition1){
if(condition2){
if(condition3){
log.info("info:{}",info);
}
}
}
}
}

Does this code make your blood pressure rise?

The code looks tangled.

How to lower the pressure? Add spaces.

Correct version:

@Service
@Slf4j
public class TestService1 {
    public void test1() {
        addLog("test1");
        if (condition1) {
            if (condition2) {
                if (condition3) {
                    log.info("info:{}", info);
                }
            }
        }
    }
}

Just adding spaces makes the hierarchy clear.

1.2 Missing Line Breaks

If necessary line breaks are missing, you might see code like:

public void update(User user) {
    if(null != user.getId()) {
        User oldUser = userMapper.findUserById(user.getId());
        if(null == oldUser)throw new RuntimeException("用户id不存在");
        oldUser.setName(user.getName());oldUser.setAge(user.getAge());oldUser.setAddress(user.getAddress());
        userMapper.updateUser(oldUser);
    } else {userMapper.insertUser(user);
    }
}

Adding spaces and line breaks makes the logic much clearer.

2. Arbitrary Naming

Java does not enforce naming rules, but careless naming leads to confusing code.

2.1 Meaningful Parameter Names

Sometimes developers use overly short names to save typing:

int a = 1;
int b = 2;
String c = "abc";
boolean b = false;

When a colleague leaves, the new developer is confused about what a, b, c mean.

Use meaningful names:

int supplierCount = 1;
int purchaserCount = 2;
String userName = "abc";
boolean hasSuccess = false;

2.2 Self‑Explanatory Names

Names should be understandable to anyone. Prefer English words over Chinese characters or pinyin.

String userName = "苏三";
String susan = "苏三";

Using common English words reduces communication cost.

2.3 Consistent Naming Style

Various naming styles (lowercase, uppercase, snake_case, camelCase) can appear in the same class, causing chaos.

// lower case
int suppliercount = 1;
// upper case
int SUPPLIERCOUNT = 1;
// lower case + underscore
int supplier_count = 1;
// upper case + underscore
int SUPPLIER_COUNT = 1;
// camelCase
int supplierCount = 1;

Recommend using camelCase for variables and parameters, and UPPER_SNAKE_CASE for static constants.

3. Massive Duplicate Code

Copy‑paste (Ctrl+C / Ctrl+V) can speed up initial development but leads to duplicated code.

Example: three services each contain an identical addLog method.

@Service
@Slf4j
public class TestService1 {
    public void test1(){ addLog("test1"); }
    private void addLog(String info){ if(log.isInfoEnabled()){ log.info("info:{}", info); } }
}

@Service
@Slf4j
public class TestService2 {
    public void test2(){ addLog("test2"); }
    private void addLog(String info){ if(log.isInfoEnabled()){ log.info("info:{}", info); } }
}

When a log‑level change is needed, all three classes must be edited, which is error‑prone.

Solution: extract the common method into a utility class.

@Slf4j
public class LogUtil {
    private LogUtil(){ throw new RuntimeException("初始化失败"); }
    public static void addLog(String info){ if(log.isDebugEnabled()){ log.debug("debug:{}", info); } }
}

Now each service calls LogUtil.addLog(...), and changes are made in one place.

4. Never Writing Comments

Under tight deadlines, developers often skip comments, believing “code is self‑documenting”.

However, without comments, future readers (including yourself) may struggle to understand complex logic.

Even core Spring methods like refresh contain extensive comments for clarity.

public void refresh() throws BeansException, IllegalStateException {
    synchronized (this.startupShutdownMonitor) {
        // Prepare this context for refreshing.
        prepareRefresh();
        // ... many more commented steps ...
    }
}

Good commenting helps maintainability and team collaboration.

5. Overly Long Methods

Long methods often contain mixed responsibilities.

public void run(){
    List<User> userList = userMapper.getAll();
    // many lines omitted
    List<User> updateList = // result
    if(CollectionUtils.isEmpty(updateList)) return;
    for(User user: updateList){
        // many lines omitted
    }
    // pagination update
    // send MQ messages
}

Refactor by extracting smaller private methods:

public void run(){
    List<User> userList = userMapper.getAll();
    List<User> updateList = filterUser(userList);
    if(CollectionUtils.isEmpty(updateList)) return;
    for(User user: updateList){ clacExpireDay(user); }
    updateUser(updateList);
    sendMq(updateList);
}

private List<User> filterUser(List<User> userList){ /* ... */ }
private void clacExpireDay(User user){ /* ... */ }
private void updateUser(List<User> updateList){ /* ... */ }
private void sendMq(List<User> updateList){ /* ... */ }

Shorter methods improve readability and reuse.

Hotspot limits JIT compilation for methods larger than 8000 bytes; such methods won’t be compiled.

6. Too Many Parameters

A method should not have more than five parameters.

public void fun(String a, String b, String c, String d, String e, String f){ ... }

Refactor by grouping related parameters into objects or result classes.

public Result fun(String a, String b, String c){ ... }
public void otherFun(Result result, String d, String e, String f){ ... }

Using Lombok’s @Builder can simplify object creation.

7. Deep Nesting

Deeply nested if or for structures are hard to read and may affect performance.

if(a==1){ if(b==2){ if(c==3){ if(d==4){ if(e==5){ ... } } } } }

Apply defensive programming: return early for negative conditions and extract inner logic into separate methods.

if(a!=1) return;
processA();

For deep loops, consider using Map to reduce nesting.

Map<Long, List<OrderDetail>> detailMap = detailList.stream().collect(Collectors.groupingBy(OrderDetail::getOrderId));
for(Order order: orderList){
    List<OrderDetail> details = detailMap.get(order.getId());
    if(CollectionUtils.isNotEmpty(details)) doSomething();
}

8. Too Many Conditional Branches

Long if…else chains for selecting payment methods violate the Open/Closed principle.

if("alia".equals(code)) aliaPay.pay();
else if("weixin".equals(code)) weixinPay.pay();
else if("jingdong".equals(code)) jingDongPay.pay();
else System.out.println("找不到支付方式");

Solution: use Strategy + Factory pattern.

public interface IPay { void pay(); }
@Service
public class AliaPay implements IPay { @PostConstruct public void init(){ PayStrategyFactory.register("aliaPay", this); } @Override public void pay(){ System.out.println("===发起支付宝支付==="); } }
// similar WeixinPay and JingDongPay
public class PayStrategyFactory {
    private static final Map<String, IPay> PAY_REGISTERS = new HashMap<>();
    public static void register(String code, IPay iPay){ if(code!=null && !"".equals(code)) PAY_REGISTERS.put(code,iPay); }
    public static IPay get(String code){ return PAY_REGISTERS.get(code); }
}
@Service
public class PayService3 { public void toPay(String code){ PayStrategyFactory.get(code).pay(); } }

This makes adding new payment methods painless.

9. Hard‑Coding Constants

Hard‑coded limits (e.g., batch size 200) require code changes and redeployment.

private static final int MAX_LIMIT = 200;
if(orderList.size() > MAX_LIMIT) throw new BusinessException("超过单次请求的数量限制");

Make them configurable via @Value or external config services.

@Value("${com.susan.maxLimit:200}")
private int maxLimit = 200;
if(orderList.size() > maxLimit) throw new BusinessException("超过单次请求的数量限制");

10. Overly Large Transactions

Annotating an entire method with @Transactional can include non‑transactional work (e.g., remote calls), causing timeouts.

@Transactional
public void updateUser(User user){
    User oldUser = userMapper.getUserById(user.getId());
    if(oldUser != null) userMapper.update(user); else userMapper.insert(user);
    sendMq(user); // should not be in the transaction
}

Use TransactionTemplate to limit the transactional block.

@Autowired
private TransactionTemplate transactionTemplate;
public void updateUser(User user){
    User oldUser = userMapper.getUserById(user.getId());
    transactionTemplate.execute(status -> {
        if(oldUser != null) userMapper.update(user); else userMapper.insert(user);
        return true;
    });
    sendMq(user);
}

11. Remote Calls Inside Loops

Calling a third‑party API inside a loop degrades performance.

for(Corp corp: corpList){
    CorpInfo info = tianyanchaService.query(corp);
    if(info == null) throw new RuntimeException("企业名称或统一社会信用代码不正确");
}

Solutions: batch remote calls if supported, or use CompletableFuture for concurrent calls.

12. Frequent Exception Catching

Manually catching exceptions just to log and rethrow adds overhead.

try{ doSameThing(); } catch(Exception e){ log.error(e.getMessage(), e); throw e; }

Prefer a global exception handler (e.g., in a gateway) to log and format responses.

13. Improper Logging

Logging full request/response payloads at INFO level can fill disks quickly.

log.info("request params:{}", ids);
log.info("response:{}", userList);

Guard with log.isDebugEnabled() so detailed logs appear only in debug mode.

14. Missing Parameter Validation

Manual validation is repetitive. Use Hibernate Validator annotations like @NotEmpty, @NotNull, and @Valid on nested objects.

@Data @NoArgsConstructor @AllArgsConstructor
public class User {
    @NotEmpty private String name;
    @NotNull private Integer age;
    @NotEmpty private String address;
    @NotNull @Valid private Role role;
}

Apply @Validated on the controller and @Valid on method parameters.

15. Inconsistent Response Formats

Different services return varied JSON structures (e.g., {"ret":0} vs {"code":0,"success":true}), causing confusion.

Standardize through a gateway that wraps all responses in a unified format.

16. Incomplete Git Commits

Submitting unfinished code (e.g., missing variable definitions) breaks the build for the whole team.

Always run the project locally before committing.

17. Unhandled Dead Code

Methods that are no longer used should be marked @Deprecated or removed to avoid confusion.

@Deprecated
public void update(User user){ System.out.println("update"); }

18. Changing Public API Signatures Arbitrarily

Renaming endpoints, parameters, or request methods without coordination breaks existing clients.

Prefer adding new APIs rather than modifying existing ones.

19. Using Map to Receive Parameters

Accepting a generic Map<String,Object> makes the contract unclear and hard to validate.

Define explicit DTO classes with validation annotations instead.

20. Never Writing Unit Tests

Skipping unit tests leads to fragile code and risky refactoring.

Even if time‑pressed, add tests later to safeguard future changes.

This article, based on real‑world experience, humorously lists common coding mistakes and offers concrete refactoring techniques, design‑pattern applications, and best‑practice recommendations for developers seeking cleaner, more maintainable code.