- Published on
Stop putting constants in catch-all classes.
The problem
At my previous gig, the outsourced IT team put in place some catch-all "constant" classes like: ValidationConstants, ErrorConstants, EventTypes.
Those "technical" classes existed solely to hold constants from across the entire codebase...
public class ValidationConstants {
public static final Set<String> VALID_ORDER_STATUSES_FOR_CANCELLATION = Set.of("PENDING", "CONFIRMED");
public static final Set<String> VALID_ORDER_STATUSES_FOR_REFUND = Set.of("SHIPPED", "DELIVERED");
public static final int MAX_INVENTORY_RESERVATION_MINUTES = 15;
public static final String SOME_UNRELATED_VALIDATION_THING = "...";
// ... 200 more constants
}
These classes have low functional cohesion. They group things by what they are (constants) rather than what they do.
A better approach
Put constants where they're used:
public class OrderCancellationService {
private static final Set<String> CANCELLABLE_STATUSES = Set.of("PENDING", "CONFIRMED");
private static final String ERROR_NOT_CANCELLABLE = "ORDER_001";
public CancellationResult cancel(Order order) {
if (!CANCELLABLE_STATUSES.contains(order.getStatus())) {
return CancellationResult.rejected(ERROR_NOT_CANCELLABLE);
}
// ...
}
}
Now OrderCancellationService is self-contained. You don't need to click through to three different files to understand what it does.
Package-by-layer vs package-by-feature
This is the same debate as package-by-layer vs package-by-feature.
Package-by-layer groups by technical role:
├── controllers/
├── services/
├── constants/ ← everything dumped here
└── mappers/
Package-by-feature groups by business capability:
├── orders/
│ ├── OrderController.java
│ ├── OrderCancellationService.java ← constants live here
│ └── OrderRefundService.java
└── inventory/
The second is more useful for the code reader because related things are together.