refactor-code

Prompt file imported from karlkyck/humansreadcode-springboot-exemplar (.github/prompts/refactor-code.prompt.md). Copyright stays with the author.

Refactor Code

You are tasked with refactoring existing code to improve maintainability, performance, and adherence to Spring Boot and Spring Modulith best practices.

Information Required

Ask for the following if not provided:

1. Code to Refactor: Specific classes, methods, or modules to refactor
2. Refactoring Goals: What improvements are needed (performance, maintainability, design)
3. Constraints: Any limitations or backward compatibility requirements
4. Priority: Which aspects are most important (security, performance, code quality)

Refactoring Categories

1. Architecture Refactoring

Extract Service Layer:

// Before: Business logic in controller
@RestController
public class OrderController {
    @Autowired
    private OrderRepository orderRepository;
    
    @PostMapping("/orders")
    public ResponseEntity<Order> createOrder(@RequestBody CreateOrderRequest request) {
        Order order = new Order();
        order.setCustomerId(request.getCustomerId());
        order.setStatus(OrderStatus.PENDING);
        order.setTotal(calculateTotal(request.getItems()));
        
        return ResponseEntity.ok(orderRepository.save(order));
    }
}

// After: Thin controller with service layer
@RestController
public class OrderController {
    private final OrderService orderService;
    
    public OrderController(OrderService orderService) {
        this.orderService = orderService;
    }
    
    @PostMapping("/orders")
    public ResponseEntity<OrderResponse> createOrder(@RequestBody OrderRequest request) {
        OrderResponse response = orderService.createOrder(request);
        return ResponseEntity.ok(response);
    }
}

@Service
public class OrderServiceImpl implements OrderService {
    private final OrderRepository orderRepository;
    
    public OrderServiceImpl(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }
    
    @Override
    @Transactional
    public OrderResponse createOrder(OrderRequest request) {
        Order order = Order.builder()
                .customerId(request.customerId())
                .status(OrderStatus.PENDING)
                .total(calculateTotal(request.items()))
                .build();
                
        Order savedOrder = orderRepository.save(order);
        return OrderResponse.from(savedOrder);
    }
}

Module Boundary Refinement:

// Before: Direct repository access across modules
@Service
public class InventoryService {
    @Autowired
    private OrderRepository orderRepository; // Wrong!
    
    public void updateStock(Long productId) {
        List<Order> orders = orderRepository.findByProductId(productId);
        // Process orders...
    }
}

// After: Event-driven communication
@Service
public class InventoryService {
    private final ApplicationEventPublisher events;
    
    public InventoryService(ApplicationEventPublisher events) {
        this.events = events;
    }
    
    @ApplicationModuleListener
    void on(OrderCreated event) {
        updateStock(event.productId());
        events.publishEvent(new StockUpdated(event.productId()));
    }
}

2. Performance Refactoring

Fix N+1 Query Problems:

// Before: N+1 queries
@Entity
public class Order {
    @OneToMany(fetch = FetchType.EAGER)
    private List<OrderItem> items; // Causes N+1
}

// After: Optimized fetching
@Entity
public class Order {
    @OneToMany(fetch = FetchType.LAZY)
    @JoinColumn(name = "order_id")
    private List<OrderItem> items;
}

// In repository
@Query("SELECT o FROM Order o JOIN FETCH o.items WHERE o.id = :id")
Optional<Order> findByIdWithItems(@Param("id") Long id);

Add Caching:

// Before: Expensive calculation on every call
@Service
public class PricingService {
    public BigDecimal calculatePrice(Long productId) {
        // Expensive calculation
        return complexPriceCalculation(productId);
    }
}

// After: Cacheable method
@Service
public class PricingService {
    @Cacheable(value = "productPrices", key = "#productId")
    public BigDecimal calculatePrice(Long productId) {
        return complexPriceCalculation(productId);
    }
    
    @CacheEvict(value = "productPrices", key = "#productId")
    public void updateProductPrice(Long productId, BigDecimal newPrice) {
        // Update price logic
    }
}

Async Processing:

// Before: Synchronous email sending
@Service
public class OrderService {
    public OrderResponse createOrder(OrderRequest request) {
        Order order = processOrder(request);
        sendConfirmationEmail(order); // Blocks request
        return OrderResponse.from(order);
    }
}

// After: Asynchronous processing
@Service
public class OrderService {
    private final ApplicationEventPublisher events;
    
    public OrderResponse createOrder(OrderRequest request) {
        Order order = processOrder(request);
        events.publishEvent(new OrderCreated(order.getId()));
        return OrderResponse.from(order);
    }
}

@Component
public class OrderEventHandler {
    @ApplicationModuleListener
    @Async
    void on(OrderCreated event) {
        sendConfirmationEmail(event.orderId());
    }
}

3. Code Quality Refactoring

Replace Magic Numbers:

// Before: Magic numbers
public class OrderService {
    public boolean isEligibleForDiscount(Order order) {
        return order.getTotal().compareTo(new BigDecimal("100.00")) > 0;
    }
}

// After: Named constants
public class OrderService {
    private static final BigDecimal DISCOUNT_THRESHOLD = new BigDecimal("100.00");
    
    public boolean isEligibleForDiscount(Order order) {
        return order.getTotal().compareTo(DISCOUNT_THRESHOLD) > 0;
    }
}

Extract Complex Methods:

// Before: Complex method
public OrderResponse processOrder(OrderRequest request) {
    // Validate request
    if (request.customerId() == null) {
        throw new ValidationException("Customer ID required");
    }
    if (request.items() == null || request.items().isEmpty()) {
        throw new ValidationException("Order items required");
    }
    
    // Calculate totals
    BigDecimal subtotal = BigDecimal.ZERO;
    for (OrderItemRequest item : request.items()) {
        BigDecimal itemTotal = item.price().multiply(new BigDecimal(item.quantity()));
        subtotal = subtotal.add(itemTotal);
    }
    BigDecimal tax = subtotal.multiply(new BigDecimal("0.08"));
    BigDecimal total = subtotal.add(tax);
    
    // Create order
    Order order = Order.builder()
            .customerId(request.customerId())
            .subtotal(subtotal)
            .tax(tax)
            .total(total)
            .status(OrderStatus.PENDING)
            .build();
    
    return OrderResponse.from(orderRepository.save(order));
}

// After: Extracted methods
public OrderResponse processOrder(OrderRequest request) {
    validateOrderRequest(request);
    OrderTotals totals = calculateOrderTotals(request.items());
    Order order = createOrder(request, totals);
    return OrderResponse.from(orderRepository.save(order));
}

private void validateOrderRequest(OrderRequest request) {
    if (request.customerId() == null) {
        throw new ValidationException("Customer ID required");
    }
    if (request.items() == null || request.items().isEmpty()) {
        throw new ValidationException("Order items required");
    }
}

private OrderTotals calculateOrderTotals(List<OrderItemRequest> items) {
    BigDecimal subtotal = items.stream()
            .map(item -> item.price().multiply(new BigDecimal(item.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
    
    BigDecimal tax = subtotal.multiply(TAX_RATE);
    BigDecimal total = subtotal.add(tax);
    
    return new OrderTotals(subtotal, tax, total);
}

Use Modern Java Features:

// Before: Traditional approach
public class OrderValidator {
    public void validateOrder(Order order) {
        if (order.getCustomerId() != null) {
            Customer customer = customerService.findById(order.getCustomerId());
            if (customer != null && customer.getStatus().equals("ACTIVE")) {
                // Process active customer
            }
        }
    }
}

// After: Modern Java with Optional and pattern matching
public class OrderValidator {
    public void validateOrder(Order order) {
        Optional.ofNullable(order.getCustomerId())
                .flatMap(customerService::findById)
                .filter(customer -> CustomerStatus.ACTIVE.equals(customer.getStatus()))
                .ifPresentOrElse(
                    this::processActiveCustomer,
                    () -> { throw new ValidationException("Customer not active"); }
                );
    }
}

4. Security Refactoring

Remove Hardcoded Secrets:

// Before: Hardcoded secrets
@Service
public class PaymentService {
    private static final String API_KEY = "sk_live_123456789"; // Bad!
    
    public PaymentResult processPayment(PaymentRequest request) {
        // Use API_KEY
    }
}

// After: Externalized configuration
@Service
public class PaymentService {
    private final PaymentProperties paymentProperties;
    
    public PaymentService(PaymentProperties paymentProperties) {
        this.paymentProperties = paymentProperties;
    }
    
    public PaymentResult processPayment(PaymentRequest request) {
        String apiKey = paymentProperties.getApiKey();
        // Use apiKey
    }
}

@ConfigurationProperties(prefix = "payment")
public record PaymentProperties(String apiKey) {}

Add Input Validation:

// Before: No validation
@RestController
public class OrderController {
    @PostMapping("/orders")
    public ResponseEntity<OrderResponse> createOrder(@RequestBody OrderRequest request) {
        return ResponseEntity.ok(orderService.createOrder(request));
    }
}

// After: Comprehensive validation
@RestController
public class OrderController {
    @PostMapping("/orders")
    public ResponseEntity<OrderResponse> createOrder(
            @Valid @RequestBody OrderRequest request) {
        return ResponseEntity.ok(orderService.createOrder(request));
    }
}

public record OrderRequest(
    @NotNull(message = "Customer ID is required")
    Long customerId,
    
    @NotEmpty(message = "Order items are required")
    @Valid
    List<OrderItemRequest> items
) {}

5. Testing Refactoring

Improve Test Quality:

// Before: Poor test structure
@Test
public void testCreateOrder() {
    Order order = orderService.createOrder(new OrderRequest(1L, List.of()));
    assertNotNull(order);
}

// After: Comprehensive testing
@Nested
@DisplayName("Order Creation")
class OrderCreationTests {
    
    @Test
    @DisplayName("Should create order successfully with valid request")
    void should_create_order_when_valid_request() {
        // Given
        OrderRequest request = OrderTestData.validOrderRequest();
        given(customerService.exists(request.customerId())).willReturn(true);
        
        // When
        OrderResponse response = orderService.createOrder(request);
        
        // Then
        assertThat(response)
                .isNotNull()
                .satisfies(order -> {
                    assertThat(order.id()).isNotNull();
                    assertThat(order.status()).isEqualTo(OrderStatus.PENDING);
                    assertThat(order.customerId()).isEqualTo(request.customerId());
                });
        
        verify(orderRepository).save(any(Order.class));
    }
    
    @Test
    @DisplayName("Should throw exception when customer ID is null")
    void should_throw_exception_when_customer_id_null() {
        // Given
        OrderRequest request = OrderTestData.orderRequestWithNullCustomerId();
        
        // When & Then
        assertThatThrownBy(() -> orderService.createOrder(request))
                .isInstanceOf(ValidationException.class)
                .hasMessage("Customer ID is required");
    }
}

Refactoring Strategy

1. Assessment Phase

• Identify code smells and anti-patterns
• Analyze performance bottlenecks
• Review security vulnerabilities
• Check adherence to project conventions

2. Planning Phase

• Prioritize refactoring tasks
• Plan incremental changes
• Identify breaking changes
• Plan testing strategy

3. Implementation Phase

• Make small, focused changes
• Maintain backward compatibility where possible
• Add comprehensive tests
• Document significant changes

4. Validation Phase

• Run full test suite
• Perform manual testing
• Check performance impact
• Review code quality metrics

Safety Guidelines

Incremental Changes:

• Make one type of change at a time
• Commit frequently with descriptive messages
• Run tests after each change
• Use feature flags for risky changes

Backward Compatibility:

• Deprecate before removing
• Provide migration guides
• Support old and new APIs temporarily
• Communicate changes to stakeholders

Testing:

• Maintain or improve test coverage
• Add tests for edge cases
• Use integration tests for complex changes
• Test performance impact

Common Refactoring Patterns

Replace Conditional with Polymorphism:

// Before
public BigDecimal calculateShipping(Order order, String shippingType) {
    if ("EXPRESS".equals(shippingType)) {
        return order.getTotal().multiply(new BigDecimal("0.15"));
    } else if ("STANDARD".equals(shippingType)) {
        return order.getTotal().multiply(new BigDecimal("0.08"));
    }
    return BigDecimal.ZERO;
}

// After
public interface ShippingCalculator {
    BigDecimal calculate(Order order);
}

@Component("EXPRESS")
public class ExpressShippingCalculator implements ShippingCalculator {
    public BigDecimal calculate(Order order) {
        return order.getTotal().multiply(new BigDecimal("0.15"));
    }
}

Extract Value Objects:

// Before
public class Order {
    private String streetAddress;
    private String city;
    private String state;
    private String zipCode;
}

// After
public record Address(
    String streetAddress,
    String city,
    String state,
    String zipCode
) {}

public class Order {
    private Address shippingAddress;
    private Address billingAddress;
}

Always ensure refactored code is more maintainable, performant, and follows project conventions while maintaining functionality.