A shipping quote service often starts simple. A user enters a package weight, a destination, a service level, and a few options. The service calculates a price and returns the result. Then the business adds express delivery, fragile handling, loyalty discounts, remote-area fees, tax rules, and email summaries. Before long, one method knows how to validate a request, calculate a price, choose a carrier, format a response, and notify the customer.
The dangerous part is not that the method is ugly. The dangerous part is that it still works. Developers become afraid to touch it because every change risks breaking a rule that is hidden somewhere in the middle. The next feature then gets added as one more if, one more temporary variable, or one more string constant. Refactoring is the way out, but the goal is not to rewrite the workflow from scratch. The goal is to improve the structure while keeping the same visible behavior.
This post walks through a practical refactoring path for a Java shipping quote workflow. The examples are small, but the order matters: first make the flow readable, then move logic to the objects that own the data, then simplify conditionals, then remove side effects and unsafe mutation. Each step should be small enough to review and test.
The Problem
Imagine a Java service that produces a delivery quote. The input is a request from a checkout page. The output is a quote with a price and a delivery message.
The service currently handles these concerns in one place:
- Request validation
- Base price calculation
- Extra fees for service level and package options
- Customer discount handling
- Carrier-specific behavior
- Missing address fallback behavior
- Email notification
A simplified flow looks like this:
Checkout UI
|
v
DeliveryQuoteService
|
+-- validates request
+-- calculates price
+-- checks service level strings
+-- reads nested address data
+-- sends email summary
|
v
QuoteResponse
The service works, but it is hard to change. Adding a new service level means searching for repeated string comparisons. Testing price calculation requires creating objects that are only needed for email sending. A missing address forces callers to do null checks. A list of package options can be modified from outside the object that owns it.
Here is a condensed version of the kind of method that causes trouble:
class DeliveryQuoteService {
private final MailNotifier mailNotifier;
DeliveryQuoteService(MailNotifier mailNotifier) {
this.mailNotifier = mailNotifier;
}
QuoteResponse createQuote(QuoteRequest request) {
if (request == null) {
throw new IllegalArgumentException("request is required");
} else {
if (request.getPackageWeightGrams() <= 0) {
throw new IllegalArgumentException("weight must be positive");
} else {
if (request.getDestination() == null) {
return new QuoteResponse("unknown", 0.0, "Missing destination");
}
}
}
double baseCharge = request.getPackageWeightGrams() * 0.004;
boolean express = request.getServiceCode().equals("EXP");
boolean overnight = request.getServiceCode().equals("OVN");
if (express || overnight) {
baseCharge = baseCharge * 1.8;
}
if (request.getOptions().contains("FRAGILE")) {
baseCharge = baseCharge + 6.50;
}
if (request.getCustomer().getTier().equals("GOLD")) {
baseCharge = baseCharge * 0.9;
}
String city = request.getDestination().getAddress().getCity().getName();
String message = "Delivery to " + city;
mailNotifier.sendQuoteSummary(request.getCustomer().getEmail(), message);
return new QuoteResponse(request.getTrackingPreview(), baseCharge, message);
}
}
This method mixes several problems:
- It has nested validation that pushes the main logic to the right.
- It uses raw strings for service levels and options.
- It exposes a message chain through
destination,address, andcity. - It mixes a query, price calculation, with a command, email sending.
- It has temporary variables that hide intent instead of explaining it.
- It returns a special response in one branch and real quote data in another.
The refactoring path below fixes these problems without changing what the service is supposed to do.
Step 1: Add Guard Clauses Before Changing the Main Logic
Nested if statements make the happy path hard to read. A guard clause handles an exceptional case early and returns or throws immediately. This keeps the main behavior flat.
QuoteResponse createQuote(QuoteRequest request) {
if (request == null) {
throw new IllegalArgumentException("request is required");
}
if (request.getPackageWeightGrams() <= 0) {
throw new IllegalArgumentException("weight must be positive");
}
if (request.getDestination() == null) {
return new QuoteResponse("unknown", 0.0, "Missing destination");
}
double baseCharge = request.getPackageWeightGrams() * 0.004;
boolean express = request.getServiceCode().equals("EXP");
boolean overnight = request.getServiceCode().equals("OVN");
if (express || overnight) {
baseCharge = baseCharge * 1.8;
}
if (request.getOptions().contains("FRAGILE")) {
baseCharge = baseCharge + 6.50;
}
if (request.getCustomer().getTier().equals("GOLD")) {
baseCharge = baseCharge * 0.9;
}
String city = request.getDestination().getAddress().getCity().getName();
String message = "Delivery to " + city;
mailNotifier.sendQuoteSummary(request.getCustomer().getEmail(), message);
return new QuoteResponse(request.getTrackingPreview(), baseCharge, message);
}
This is not a redesign yet. It is a readability refactor. The important improvement is that validation is now separated from the normal path. A reviewer can confirm that the same checks still happen, but the main flow is easier to follow.
Step 2: Extract Methods Around Intent, Not Mechanics
A long method usually contains smaller ideas. The mistake is extracting code based only on line count. A better rule is to extract a method when you can name the intention clearly.
In the quote method, these ideas are visible:
- Validate the request
- Calculate the charge
- Build the delivery message
- Notify the customer
- Build the response
QuoteResponse createQuote(QuoteRequest request) {
validateRequest(request);
Money totalCharge = calculateCharge(request);
String message = deliveryMessageFor(request);
mailNotifier.sendQuoteSummary(request.getCustomer().getEmail(), message);
return new QuoteResponse(request.getTrackingPreview(), totalCharge.amount(), message);
}
private void validateRequest(QuoteRequest request) {
if (request == null) {
throw new IllegalArgumentException("request is required");
}
if (request.getPackageWeightGrams() <= 0) {
throw new IllegalArgumentException("weight must be positive");
}
if (request.getDestination() == null) {
throw new IllegalArgumentException("destination is required");
}
}
private String deliveryMessageFor(QuoteRequest request) {
return "Delivery to " + request.getDestination().getCityName();
}
Notice the method names. They say what the code is doing, not how it is doing it. calculateCharge is more useful than multiplyWeightAndAddFees. deliveryMessageFor is more useful than concatenateCityString.
This also exposes the next design problem: createQuote still notifies the customer while calculating a quote. That side effect will be handled later.
Step 3: Replace Temporary Variables With Meaningful Queries
Temporary variables are not always bad. They can improve readability when they name a concept. They become a problem when they only store a calculation that belongs somewhere else.
In the original code, express and overnight exist only because the service level is represented as a string. We can make the condition more meaningful before introducing a larger design change.
private Money calculateCharge(QuoteRequest request) {
Money charge = baseChargeFor(request);
if (usesPremiumSpeed(request)) {
charge = charge.multiplyBy(1.8);
}
if (requiresFragileHandling(request)) {
charge = charge.add(Money.usd(6.50));
}
if (isGoldCustomer(request)) {
charge = charge.multiplyBy(0.9);
}
return charge;
}
private boolean usesPremiumSpeed(QuoteRequest request) {
return request.getServiceCode().equals("EXP")
|| request.getServiceCode().equals("OVN");
}
private boolean requiresFragileHandling(QuoteRequest request) {
return request.getOptions().contains("FRAGILE");
}
private boolean isGoldCustomer(QuoteRequest request) {
return request.getCustomer().getTier().equals("GOLD");
}
The behavior is still the same, but the calculation now reads more like a pricing policy. This kind of extraction is useful as an intermediate step. It makes the next refactor safer because the rules are named and isolated.
Step 4: Replace Primitive Values With Domain Types
Raw strings make code easy to misuse. "EXP", "OVN", "FRAGILE", and "GOLD" are valid only because developers remember the convention. A typo still compiles. A swapped value still compiles. A method can accept a service code where it should accept a customer tier.
A simple improvement is to replace string constants with enums when the possible values are fixed.
enum ServiceLevel {
STANDARD,
EXPRESS,
OVERNIGHT;
boolean hasPremiumSpeed() {
return this == EXPRESS || this == OVERNIGHT;
}
}
enum PackageOption {
FRAGILE,
SIGNATURE_REQUIRED
}
enum CustomerTier {
STANDARD,
GOLD
}
Now the request can expose meaningful types:
class QuoteRequest {
private final int packageWeightGrams;
private final ServiceLevel serviceLevel;
private final List<PackageOption> options;
private final Customer customer;
private final Destination destination;
QuoteRequest(
int packageWeightGrams,
ServiceLevel serviceLevel,
List<PackageOption> options,
Customer customer,
Destination destination
) {
this.packageWeightGrams = packageWeightGrams;
this.serviceLevel = serviceLevel;
this.options = List.copyOf(options);
this.customer = customer;
this.destination = destination;
}
List<PackageOption> getOptions() {
return Collections.unmodifiableList(options);
}
boolean hasOption(PackageOption option) {
return options.contains(option);
}
ServiceLevel getServiceLevel() {
return serviceLevel;
}
}
Two things happen here. First, the compiler now protects you from passing random strings. Second, the options list is no longer directly modifiable by callers. The object that owns the collection keeps control of it.
The calculation becomes simpler:
private Money calculateCharge(QuoteRequest request) {
Money charge = baseChargeFor(request);
if (request.getServiceLevel().hasPremiumSpeed()) {
charge = charge.multiplyBy(1.8);
}
if (request.hasOption(PackageOption.FRAGILE)) {
charge = charge.add(Money.usd(6.50));
}
if (request.getCustomer().isGold()) {
charge = charge.multiplyBy(0.9);
}
return charge;
}
The service now asks the domain objects questions instead of parsing their internals.
Step 5: Move Behavior Toward the Data It Uses
When a method repeatedly pulls data out of another object, the method may be in the wrong place. Price rules that depend on service level can move into ServiceLevel. Discount rules that depend on the customer can move into Customer or a dedicated discount policy.
For example, ServiceLevel can own its price multiplier:
enum ServiceLevel {
STANDARD(1.0),
EXPRESS(1.8),
OVERNIGHT(1.8);
private final double multiplier;
ServiceLevel(double multiplier) {
this.multiplier = multiplier;
}
Money applyTo(Money baseCharge) {
return baseCharge.multiplyBy(multiplier);
}
}
Then the quote service no longer needs to know which levels are premium:
private Money calculateCharge(QuoteRequest request) {
Money charge = baseChargeFor(request);
charge = request.getServiceLevel().applyTo(charge);
if (request.hasOption(PackageOption.FRAGILE)) {
charge = charge.add(Money.usd(6.50));
}
return request.getCustomer().applyDiscountTo(charge);
}
This is a small example of moving a method or field to a better home. The rule of thumb is practical: place behavior close to the data it uses most. That reduces repeated getter calls and makes future changes more local.
Step 6: Use Polymorphism When Type Checks Keep Growing
Enums are useful when a fixed list of values is enough. If each service level starts to require different calculations, different validation rules, or different carrier behavior, repeated conditionals may return.
At that point, use polymorphism. Define a common abstraction and let each implementation handle its own rule.
interface DeliveryPricingPlan {
Money price(Money baseCharge);
}
final class StandardPricingPlan implements DeliveryPricingPlan {
public Money price(Money baseCharge) {
return baseCharge;
}
}
final class ExpressPricingPlan implements DeliveryPricingPlan {
public Money price(Money baseCharge) {
return baseCharge.multiplyBy(1.8);
}
}
final class OvernightPricingPlan implements DeliveryPricingPlan {
public Money price(Money baseCharge) {
return baseCharge.multiplyBy(1.8).add(Money.usd(4.00));
}
}
The caller can now depend on the interface:
private Money calculateCharge(QuoteRequest request) {
Money charge = baseChargeFor(request);
charge = request.getPricingPlan().price(charge);
if (request.hasOption(PackageOption.FRAGILE)) {
charge = charge.add(Money.usd(6.50));
}
return request.getCustomer().applyDiscountTo(charge);
}
This follows a useful design direction: existing code stays closed for direct modification, while new behavior can be added through new implementations. Do not use polymorphism just to avoid one small if. Use it when the same type check appears repeatedly or when each type is gaining its own behavior.
Step 7: Hide Message Chains Behind Useful Methods
A message chain is a call such as this:
String city = request.getDestination().getAddress().getCity().getName();
The quote service now knows too much about the internal shape of Destination. If Address changes, the quote service may need to change even though it only wanted a city name.
A simple delegate method hides that structure:
class Destination {
private final Address address;
Destination(Address address) {
this.address = address;
}
String cityName() {
return address.cityName();
}
}
class Address {
private final City city;
Address(City city) {
this.city = city;
}
String cityName() {
return city.name();
}
}
Now the service says what it needs:
private String deliveryMessageFor(QuoteRequest request) {
return "Delivery to " + request.getDestination().cityName();
}
Be careful not to create useless pass-through methods everywhere. A delegate method is helpful when it hides a real implementation detail or reduces coupling. If it only forwards calls without adding clarity, it can become a middleman.
Step 8: Return a Special Case Instead of Forcing Null Checks
A method that returns null pushes conditional logic onto every caller. In a quote workflow, a missing destination might be common enough to model explicitly.
Define an interface and two implementations:
interface QuoteDestination {
String cityName();
boolean isKnown();
}
final class KnownDestination implements QuoteDestination {
private final Address address;
KnownDestination(Address address) {
this.address = address;
}
public String cityName() {
return address.cityName();
}
public boolean isKnown() {
return true;
}
}
final class UnknownDestination implements QuoteDestination {
public String cityName() {
return "unknown";
}
public boolean isKnown() {
return false;
}
}
Now deliveryMessageFor no longer needs a null branch:
private String deliveryMessageFor(QuoteDestination destination) {
return "Delivery to " + destination.cityName();
}
This is not always the right answer. Sometimes an exception is clearer. Sometimes Optional is acceptable. A special case object works well when the caller can continue with safe default behavior.
Step 9: Separate Queries From Commands
The original createQuote method calculated a quote and sent an email. That is a side effect. A caller might reasonably expect that asking for a quote only returns information. They may not expect it to notify the customer.
Separate the query from the command:
QuoteResponse createQuote(QuoteRequest request) {
validateRequest(request);
Money totalCharge = calculateCharge(request);
String message = deliveryMessageFor(request.getDestination());
return new QuoteResponse(request.getTrackingPreview(), totalCharge.amount(), message);
}
void sendQuoteSummary(Customer customer, QuoteResponse quote) {
mailNotifier.sendQuoteSummary(customer.getEmail(), quote.message());
}
Now price calculation can be called repeatedly without changing system state. Email notification is still available, but the caller chooses when to perform it. This makes testing easier because tests for pricing no longer need to verify or mock email behavior.
Step 10: Remove Setters When Objects Should Not Change
Mutable objects are easy to misconfigure. If a request can be created empty and populated through setters, it can also be sent halfway through construction.
Prefer constructors or builders when an object needs multiple fields. Here is a compact builder for the quote request:
final class QuoteRequestBuilder {
private int packageWeightGrams;
private ServiceLevel serviceLevel = ServiceLevel.STANDARD;
private List<PackageOption> options = List.of();
private Customer customer;
private QuoteDestination destination = new UnknownDestination();
QuoteRequestBuilder weightInGrams(int packageWeightGrams) {
this.packageWeightGrams = packageWeightGrams;
return this;
}
QuoteRequestBuilder serviceLevel(ServiceLevel serviceLevel) {
this.serviceLevel = serviceLevel;
return this;
}
QuoteRequestBuilder options(List<PackageOption> options) {
this.options = List.copyOf(options);
return this;
}
QuoteRequestBuilder customer(Customer customer) {
this.customer = customer;
return this;
}
QuoteRequestBuilder destination(QuoteDestination destination) {
this.destination = destination;
return this;
}
QuoteRequest build() {
return new QuoteRequest(packageWeightGrams, serviceLevel, options, customer, destination);
}
}
A builder can be useful when the constructor would otherwise have too many parameters or when some values are optional. The resulting object should still protect its state with private final fields and defensive copies for mutable collections.
Common Mistakes
Extracting tiny methods without improving intent
A method such as multiplyByTwo is not always better than the expression itself. Extract methods when the name explains a business concept, removes duplication, or creates a useful seam for future change.
Moving logic before understanding dependencies
Before moving a method to another class, check what it reads and what it changes. If moving it requires passing half the old object as parameters, the target may be wrong.
Replacing every conditional with inheritance
Polymorphism is useful when behavior varies by type and keeps growing. A single clear guard clause or a small if can be fine.
Returning mutable collections
A getter that returns the real List lets callers change an object without using that object's methods. Return an unmodifiable view or a defensive copy, and provide explicit add or remove operations only when mutation is intended.
Keeping dead code because it might be useful later
Dead code adds noise and increases maintenance cost. A version control system can recover old code if needed. Commented-out blocks and unused helpers should be removed.
Refactoring Checklist
Use this checklist when cleaning up a Java workflow like the shipping quote service:
- Confirm the current behavior with tests or examples before changing structure.
- Flatten exceptional cases with guard clauses.
- Extract methods around business intent.
- Inline one-line methods or variables that add no clarity.
- Replace raw strings and numbers with enums or small domain objects when they represent business concepts.
- Move behavior toward the class that owns the data it uses.
- Hide message chains when callers do not need to know the internal object graph.
- Use special case objects when safe defaults are clearer than repeated
nullchecks. - Separate queries from commands so read operations do not cause surprising side effects.
- Remove setters when construction should produce a valid object.
- Delete dead code instead of preserving it as comments.
- Run the test suite after every small step.
Conclusion
Refactoring a fragile Java service is not a single dramatic rewrite. It is a sequence of small, behavior-preserving moves. Guard clauses make the happy path visible. Extracted methods turn hidden intent into named steps. Domain types replace unsafe strings. Polymorphism removes repeated type checks when behavior varies. Encapsulation protects object state, and separating queries from commands removes surprising side effects.
The practical goal is simple: the shipping quote service should still return the same quote for the same input, but the next developer should be able to add a rule, change a service level, or test a calculation without reading one large method from top to bottom.
